Message ID | 20240729103539.35608-2-ghadi.rahme@canonical.com |
---|---|
State | New |
Headers | show |
Series | bnx2x: Fix multiple UBSAN array-index-out-of-bounds | expand |
On Mon, Jul 29, 2024 at 6:37 PM Ghadi Elie Rahme <ghadi.rahme@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@canonical.com> > Cc: stable@vger.kernel.org > Link: https://patch.msgid.link/20240627111405.1037812-1-ghadi.rahme@canonical.com > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > (cherry picked from commit 134061163ee5ca4759de5c24ca3bd71608891ba7) > signed-off-by: Ghadi Elie Rahme <ghadi.rahme@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@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team Acked-by: Chris Chiu <chris.chiu@canonical.com>
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]; };