On Fri, Jan 13, 2023 at 02:44:32PM -0800, Kees Cook wrote:
> On Fri, Jan 13, 2023 at 04:08:21PM +0000, Niklas Cassel wrote:
> > > Hello Kees,
> > >
> > > Unfortunately, this commit introduces a crash in the bnxt
> > > ethernet driver when booting linux-next.
(snip)
>
> Let's see...
>
> struct hwrm_selftest_qlist_output {
> ...
> 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];
> ...
> };
>
> Ew. So, yes, it's specifically reach past the end of the test0_name[]
> array, *and* is may overflow the heap. Does this patch solve it for you?
Yes, it does!
Thank you very much Kees, both for this patch, and for all the excellent work
that you've done with regard to kernel hardening in general over the years.
Feel free to add my:
Tested-by: Niklas Cassel <niklas.cassel@xxxxxxx>
if you send out a real patch.
Kind regards,
Niklas
>
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index cbf17fcfb7ab..ec573127b707 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -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");
> }
> }
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
> index 2686a714a59f..a5408879e077 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
> @@ -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
>
>
>
>
>
> --
> Kees Cook
