Commit d3e599c0 authored by Kees Cook's avatar Kees Cook Committed by David S. Miller

bnxt: Do not read past the end of test names

Test names were being concatenated based on a offset beyond the end of
the first name, which tripped the buffer overflow detection logic:

 detected buffer overflow in strnlen
 [...]
 Call Trace:
 bnxt_ethtool_init.cold+0x18/0x18

Refactor struct hwrm_selftest_qlist_output to use an actual array,
and adjust the concatenation to use snprintf() rather than a series of
strncat() calls.
Reported-by: default avatarNiklas Cassel <Niklas.Cassel@wdc.com>
Link: https://lore.kernel.org/lkml/Y8F%2F1w1AZTvLglFX@x1-carbon/Tested-by: default avatarNiklas Cassel <Niklas.Cassel@wdc.com>
Fixes: eb513658 ("bnxt_en: Add basic ethtool -t selftest support.")
Cc: Michael Chan <michael.chan@broadcom.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Reviewed-by: default avatarMichael Chan <michael.chan@broadcom.com>
Reviewed-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent fdfc76a1
...@@ -3969,7 +3969,7 @@ void bnxt_ethtool_init(struct bnxt *bp) ...@@ -3969,7 +3969,7 @@ void bnxt_ethtool_init(struct bnxt *bp)
test_info->timeout = HWRM_CMD_TIMEOUT; test_info->timeout = HWRM_CMD_TIMEOUT;
for (i = 0; i < bp->num_tests; i++) { for (i = 0; i < bp->num_tests; i++) {
char *str = test_info->string[i]; char *str = test_info->string[i];
char *fw_str = resp->test0_name + i * 32; char *fw_str = resp->test_name[i];
if (i == BNXT_MACLPBK_TEST_IDX) { if (i == BNXT_MACLPBK_TEST_IDX) {
strcpy(str, "Mac loopback test (offline)"); strcpy(str, "Mac loopback test (offline)");
...@@ -3980,14 +3980,9 @@ void bnxt_ethtool_init(struct bnxt *bp) ...@@ -3980,14 +3980,9 @@ void bnxt_ethtool_init(struct bnxt *bp)
} else if (i == BNXT_IRQ_TEST_IDX) { } else if (i == BNXT_IRQ_TEST_IDX) {
strcpy(str, "Interrupt_test (offline)"); strcpy(str, "Interrupt_test (offline)");
} else { } else {
strscpy(str, fw_str, ETH_GSTRING_LEN); snprintf(str, ETH_GSTRING_LEN, "%s test (%s)",
strncat(str, " test", ETH_GSTRING_LEN - strlen(str)); fw_str, test_info->offline_mask & (1 << i) ?
if (test_info->offline_mask & (1 << i)) "offline" : "online");
strncat(str, " (offline)",
ETH_GSTRING_LEN - strlen(str));
else
strncat(str, " (online)",
ETH_GSTRING_LEN - strlen(str));
} }
} }
......
...@@ -10249,14 +10249,7 @@ struct hwrm_selftest_qlist_output { ...@@ -10249,14 +10249,7 @@ struct hwrm_selftest_qlist_output {
u8 unused_0; u8 unused_0;
__le16 test_timeout; __le16 test_timeout;
u8 unused_1[2]; u8 unused_1[2];
char test0_name[32]; char test_name[8][32];
char test1_name[32];
char test2_name[32];
char test3_name[32];
char test4_name[32];
char test5_name[32];
char test6_name[32];
char test7_name[32];
u8 eyescope_target_BER_support; u8 eyescope_target_BER_support;
#define SELFTEST_QLIST_RESP_EYESCOPE_TARGET_BER_SUPPORT_BER_1E8_SUPPORTED 0x0UL #define SELFTEST_QLIST_RESP_EYESCOPE_TARGET_BER_SUPPORT_BER_1E8_SUPPORTED 0x0UL
#define SELFTEST_QLIST_RESP_EYESCOPE_TARGET_BER_SUPPORT_BER_1E9_SUPPORTED 0x1UL #define SELFTEST_QLIST_RESP_EYESCOPE_TARGET_BER_SUPPORT_BER_1E9_SUPPORTED 0x1UL
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment