ACK: [PATCH 1/1] bnx2x: Fix multiple UBSAN array-index-out-of-bounds
Chris Chiu
chris.chiu at canonical.com
Tue Jul 30 02:30:10 UTC 2024
On Mon, Jul 29, 2024 at 6:37 PM Ghadi Elie Rahme
<ghadi.rahme at canonical.com> wrote:
>
> Buglink: https://bugs.launchpad.net/bugs/2074215
>
> Fix UBSAN warnings that occur when using a system with 32 physical
> cpu cores or more, or when the user defines a number of Ethernet
> queues greater than or equal to FP_SB_MAX_E1x using the num_queues
> module parameter.
>
> Currently there is a read/write out of bounds that occurs on the array
> "struct stats_query_entry query" present inside the "bnx2x_fw_stats_req"
> struct in "drivers/net/ethernet/broadcom/bnx2x/bnx2x.h".
> Looking at the definition of the "struct stats_query_entry query" array:
>
> struct stats_query_entry query[FP_SB_MAX_E1x+
> BNX2X_FIRST_QUEUE_QUERY_IDX];
>
> FP_SB_MAX_E1x is defined as the maximum number of fast path interrupts and
> has a value of 16, while BNX2X_FIRST_QUEUE_QUERY_IDX has a value of 3
> meaning the array has a total size of 19.
> Since accesses to "struct stats_query_entry query" are offset-ted by
> BNX2X_FIRST_QUEUE_QUERY_IDX, that means that the total number of Ethernet
> queues should not exceed FP_SB_MAX_E1x (16). However one of these queues
> is reserved for FCOE and thus the number of Ethernet queues should be set
> to [FP_SB_MAX_E1x -1] (15) if FCOE is enabled or [FP_SB_MAX_E1x] (16) if
> it is not.
>
> This is also described in a comment in the source code in
> drivers/net/ethernet/broadcom/bnx2x/bnx2x.h just above the Macro definition
> of FP_SB_MAX_E1x. Below is the part of this explanation that it important
> for this patch
>
> /*
> * The total number of L2 queues, MSIX vectors and HW contexts (CIDs) is
> * control by the number of fast-path status blocks supported by the
> * device (HW/FW). Each fast-path status block (FP-SB) aka non-default
> * status block represents an independent interrupts context that can
> * serve a regular L2 networking queue. However special L2 queues such
> * as the FCoE queue do not require a FP-SB and other components like
> * the CNIC may consume FP-SB reducing the number of possible L2 queues
> *
> * If the maximum number of FP-SB available is X then:
> * a. If CNIC is supported it consumes 1 FP-SB thus the max number of
> * regular L2 queues is Y=X-1
> * b. In MF mode the actual number of L2 queues is Y= (X-1/MF_factor)
> * c. If the FCoE L2 queue is supported the actual number of L2 queues
> * is Y+1
> * d. The number of irqs (MSIX vectors) is either Y+1 (one extra for
> * slow-path interrupts) or Y+2 if CNIC is supported (one additional
> * FP interrupt context for the CNIC).
> * e. The number of HW context (CID count) is always X or X+1 if FCoE
> * L2 queue is supported. The cid for the FCoE L2 queue is always X.
> */
>
> However this driver also supports NICs that use the E2 controller which can
> handle more queues due to having more FP-SB represented by FP_SB_MAX_E2.
> Looking at the commits when the E2 support was added, it was originally
> using the E1x parameters: commit f2e0899f0f27 ("bnx2x: Add 57712 support").
> Back then FP_SB_MAX_E2 was set to 16 the same as E1x. However the driver
> was later updated to take full advantage of the E2 instead of having it be
> limited to the capabilities of the E1x. But as far as we can tell, the
> array "stats_query_entry query" was still limited to using the FP-SB
> available to the E1x cards as part of an oversignt when the driver was
> updated to take full advantage of the E2, and now with the driver being
> aware of the greater queue size supported by E2 NICs, it causes the UBSAN
> warnings seen in the stack traces below.
>
> This patch increases the size of the "stats_query_entry query" array by
> replacing FP_SB_MAX_E1x with FP_SB_MAX_E2 to be large enough to handle
> both types of NICs.
>
> Stack traces:
>
> UBSAN: array-index-out-of-bounds in
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c:1529:11
> index 20 is out of range for type 'stats_query_entry [19]'
> CPU: 12 PID: 858 Comm: systemd-network Not tainted 6.9.0-060900rc7-generic
> #202405052133
> Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9,
> BIOS P89 10/21/2019
> Call Trace:
> <TASK>
> dump_stack_lvl+0x76/0xa0
> dump_stack+0x10/0x20
> __ubsan_handle_out_of_bounds+0xcb/0x110
> bnx2x_prep_fw_stats_req+0x2e1/0x310 [bnx2x]
> bnx2x_stats_init+0x156/0x320 [bnx2x]
> bnx2x_post_irq_nic_init+0x81/0x1a0 [bnx2x]
> bnx2x_nic_load+0x8e8/0x19e0 [bnx2x]
> bnx2x_open+0x16b/0x290 [bnx2x]
> __dev_open+0x10e/0x1d0
> RIP: 0033:0x736223927a0a
> Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca
> 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00
> f0 ff ff 77 7e c3 0f 1f 44 00 00 41 54 48 83 ec 30 44 89
> RSP: 002b:00007ffc0bb2ada8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
> RAX: ffffffffffffffda RBX: 0000583df50f9c78 RCX: 0000736223927a0a
> RDX: 0000000000000020 RSI: 0000583df50ee510 RDI: 0000000000000003
> RBP: 0000583df50d4940 R08: 00007ffc0bb2adb0 R09: 0000000000000080
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000583df5103ae0
> R13: 000000000000035a R14: 0000583df50f9c30 R15: 0000583ddddddf00
> </TASK>
> ---[ end trace ]---
> ------------[ cut here ]------------
> UBSAN: array-index-out-of-bounds in
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c:1546:11
> index 28 is out of range for type 'stats_query_entry [19]'
> CPU: 12 PID: 858 Comm: systemd-network Not tainted 6.9.0-060900rc7-generic
> #202405052133
> Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9,
> BIOS P89 10/21/2019
> Call Trace:
> <TASK>
> dump_stack_lvl+0x76/0xa0
> dump_stack+0x10/0x20
> __ubsan_handle_out_of_bounds+0xcb/0x110
> bnx2x_prep_fw_stats_req+0x2fd/0x310 [bnx2x]
> bnx2x_stats_init+0x156/0x320 [bnx2x]
> bnx2x_post_irq_nic_init+0x81/0x1a0 [bnx2x]
> bnx2x_nic_load+0x8e8/0x19e0 [bnx2x]
> bnx2x_open+0x16b/0x290 [bnx2x]
> __dev_open+0x10e/0x1d0
> RIP: 0033:0x736223927a0a
> Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca
> 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00
> f0 ff ff 77 7e c3 0f 1f 44 00 00 41 54 48 83 ec 30 44 89
> RSP: 002b:00007ffc0bb2ada8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
> RAX: ffffffffffffffda RBX: 0000583df50f9c78 RCX: 0000736223927a0a
> RDX: 0000000000000020 RSI: 0000583df50ee510 RDI: 0000000000000003
> RBP: 0000583df50d4940 R08: 00007ffc0bb2adb0 R09: 0000000000000080
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000583df5103ae0
> R13: 000000000000035a R14: 0000583df50f9c30 R15: 0000583ddddddf00
> </TASK>
> ---[ end trace ]---
> ------------[ cut here ]------------
> UBSAN: array-index-out-of-bounds in
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c:1895:8
> index 29 is out of range for type 'stats_query_entry [19]'
> CPU: 13 PID: 163 Comm: kworker/u96:1 Not tainted 6.9.0-060900rc7-generic
> #202405052133
> Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9,
> BIOS P89 10/21/2019
> Workqueue: bnx2x bnx2x_sp_task [bnx2x]
> Call Trace:
> <TASK>
> dump_stack_lvl+0x76/0xa0
> dump_stack+0x10/0x20
> __ubsan_handle_out_of_bounds+0xcb/0x110
> bnx2x_iov_adjust_stats_req+0x3c4/0x3d0 [bnx2x]
> bnx2x_storm_stats_post.part.0+0x4a/0x330 [bnx2x]
> ? bnx2x_hw_stats_post+0x231/0x250 [bnx2x]
> bnx2x_stats_start+0x44/0x70 [bnx2x]
> bnx2x_stats_handle+0x149/0x350 [bnx2x]
> bnx2x_attn_int_asserted+0x998/0x9b0 [bnx2x]
> bnx2x_sp_task+0x491/0x5c0 [bnx2x]
> process_one_work+0x18d/0x3f0
> </TASK>
> ---[ end trace ]---
>
> Fixes: 50f0a562f8cc ("bnx2x: add fcoe statistics")
> Signed-off-by: Ghadi Elie Rahme <ghadi.rahme at canonical.com>
> Cc: stable at vger.kernel.org
> Link: https://patch.msgid.link/20240627111405.1037812-1-ghadi.rahme@canonical.com
> Signed-off-by: Jakub Kicinski <kuba at kernel.org>
> (cherry picked from commit 134061163ee5ca4759de5c24ca3bd71608891ba7)
> signed-off-by: Ghadi Elie Rahme <ghadi.rahme at canonical.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> index e2a4e1088b7f..9580ab83d387 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> @@ -1262,7 +1262,7 @@ enum {
>
> struct bnx2x_fw_stats_req {
> struct stats_query_header hdr;
> - struct stats_query_entry query[FP_SB_MAX_E1x+
> + struct stats_query_entry query[FP_SB_MAX_E2 +
> BNX2X_FIRST_QUEUE_QUERY_IDX];
> };
>
> --
> 2.43.0
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Acked-by: Chris Chiu <chris.chiu at canonical.com>
More information about the kernel-team
mailing list