bnxt: Do not read past the end of test names
authorKees Cook <keescook@chromium.org>
Wed, 18 Jan 2023 20:35:01 +0000 (12:35 -0800)
committerDavid S. Miller <davem@davemloft.net>
Fri, 20 Jan 2023 12:52:29 +0000 (12:52 +0000)
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: Niklas Cassel <Niklas.Cassel@wdc.com>
Link: https://lore.kernel.org/lkml/Y8F%2F1w1AZTvLglFX@x1-carbon/
Tested-by: Niklas Cassel <Niklas.Cassel@wdc.com>
Fixes: eb51365846bc ("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: Kees Cook <keescook@chromium.org>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h

index cbf17fc..ec57312 100644 (file)
@@ -3969,7 +3969,7 @@ void bnxt_ethtool_init(struct bnxt *bp)
                test_info->timeout = HWRM_CMD_TIMEOUT;
        for (i = 0; i < bp->num_tests; 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) {
                        strcpy(str, "Mac loopback test (offline)");
@@ -3980,14 +3980,9 @@ void bnxt_ethtool_init(struct bnxt *bp)
                } else if (i == BNXT_IRQ_TEST_IDX) {
                        strcpy(str, "Interrupt_test (offline)");
                } else {
-                       strscpy(str, fw_str, ETH_GSTRING_LEN);
-                       strncat(str, " test", ETH_GSTRING_LEN - strlen(str));
-                       if (test_info->offline_mask & (1 << i))
-                               strncat(str, " (offline)",
-                                       ETH_GSTRING_LEN - strlen(str));
-                       else
-                               strncat(str, " (online)",
-                                       ETH_GSTRING_LEN - strlen(str));
+                       snprintf(str, ETH_GSTRING_LEN, "%s test (%s)",
+                                fw_str, test_info->offline_mask & (1 << i) ?
+                                       "offline" : "online");
                }
        }
 
index 2686a71..a540887 100644 (file)
@@ -10249,14 +10249,7 @@ struct hwrm_selftest_qlist_output {
        u8      unused_0;
        __le16  test_timeout;
        u8      unused_1[2];
-       char    test0_name[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];
+       char    test_name[8][32];
        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_1E9_SUPPORTED  0x1UL