| Message ID | 20220208001918.900602-1-sukadev@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 61772b0908c640d0309c40f7d41d062ca4e979fa |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/1] ibmvnic: don't release napi in __ibmvnic_open() | expand |
On 2022-02-07 16:19, Sukadev Bhattiprolu wrote: > If __ibmvnic_open() encounters an error such as when setting link > state, > it calls release_resources() which frees the napi structures > needlessly. > Instead, have __ibmvnic_open() only clean up the work it did so far > (i.e. > disable napi and irqs) and leave the rest to the callers. > > If caller of __ibmvnic_open() is ibmvnic_open(), it should release the > resources immediately. If the caller is do_reset() or do_hard_reset(), > they will release the resources on the next reset. > > This fixes following crash that occured when running the drmgr command > several times to add/remove a vnic interface: > > [102056] ibmvnic 30000003 env3: Disabling rx_scrq[6] irq > [102056] ibmvnic 30000003 env3: Disabling rx_scrq[7] irq > [102056] ibmvnic 30000003 env3: Replenished 8 pools > Kernel attempted to read user page (10) - exploit attempt? (uid: 0) > BUG: Kernel NULL pointer dereference on read at 0x00000010 > Faulting instruction address: 0xc000000000a3c840 > Oops: Kernel access of bad area, sig: 11 [#1] > LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries > ... > CPU: 9 PID: 102056 Comm: kworker/9:2 Kdump: loaded Not tainted > 5.16.0-rc5-autotest-g6441998e2e37 #1 > Workqueue: events_long __ibmvnic_reset [ibmvnic] > NIP: c000000000a3c840 LR: c0080000029b5378 CTR: c000000000a3c820 > REGS: c0000000548e37e0 TRAP: 0300 Not tainted > (5.16.0-rc5-autotest-g6441998e2e37) > MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 28248484 XER: > 00000004 > CFAR: c0080000029bdd24 DAR: 0000000000000010 DSISR: 40000000 IRQMASK: > 0 > GPR00: c0080000029b55d0 c0000000548e3a80 c0000000028f0200 > 0000000000000000 > ... > NIP [c000000000a3c840] napi_enable+0x20/0xc0 > LR [c0080000029b5378] __ibmvnic_open+0xf0/0x430 [ibmvnic] > Call Trace: > [c0000000548e3a80] [0000000000000006] 0x6 (unreliable) > [c0000000548e3ab0] [c0080000029b55d0] __ibmvnic_open+0x348/0x430 > [ibmvnic] > [c0000000548e3b40] [c0080000029bcc28] __ibmvnic_reset+0x500/0xdf0 > [ibmvnic] > [c0000000548e3c60] [c000000000176228] process_one_work+0x288/0x570 > [c0000000548e3d00] [c000000000176588] worker_thread+0x78/0x660 > [c0000000548e3da0] [c0000000001822f0] kthread+0x1c0/0x1d0 > [c0000000548e3e10] [c00000000000cf64] ret_from_kernel_thread+0x5c/0x64 > Instruction dump: > 7d2948f8 792307e0 4e800020 60000000 3c4c01eb 384239e0 f821ffd1 > 39430010 > 38a0fff6 e92d1100 f9210028 39200000 <e9030010> f9010020 60420000 > e9210020 > ---[ end trace 5f8033b08fd27706 ]--- > > Fixes: ed651a10875f ("ibmvnic: Updated reset handling) > Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> Reviewed-by: Dany Madden <drt@linux.ibm.com> > --- > drivers/net/ethernet/ibm/ibmvnic.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c > b/drivers/net/ethernet/ibm/ibmvnic.c > index f5d1ba9bea48..50d2e48274eb 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -110,6 +110,7 @@ static void ibmvnic_tx_scrq_clean_buffer(struct > ibmvnic_adapter *adapter, > struct ibmvnic_sub_crq_queue *tx_scrq); > static void free_long_term_buff(struct ibmvnic_adapter *adapter, > struct ibmvnic_long_term_buff *ltb); > +static void ibmvnic_disable_irqs(struct ibmvnic_adapter *adapter); > > struct ibmvnic_stat { > char name[ETH_GSTRING_LEN]; > @@ -1424,7 +1425,7 @@ static int __ibmvnic_open(struct net_device > *netdev) > rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP); > if (rc) { > ibmvnic_napi_disable(adapter); > - release_resources(adapter); > + ibmvnic_disable_irqs(adapter); > return rc; > } > > @@ -1474,9 +1475,6 @@ static int ibmvnic_open(struct net_device > *netdev) > rc = init_resources(adapter); > if (rc) { > netdev_err(netdev, "failed to initialize resources\n"); > - release_resources(adapter); > - release_rx_pools(adapter); > - release_tx_pools(adapter); > goto out; > } > } > @@ -1493,6 +1491,13 @@ static int ibmvnic_open(struct net_device > *netdev) > adapter->state = VNIC_OPEN; > rc = 0; > } > + > + if (rc) { > + release_resources(adapter); > + release_rx_pools(adapter); > + release_tx_pools(adapter); > + } > + > return rc; > }
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Mon, 7 Feb 2022 16:19:18 -0800 you wrote: > If __ibmvnic_open() encounters an error such as when setting link state, > it calls release_resources() which frees the napi structures needlessly. > Instead, have __ibmvnic_open() only clean up the work it did so far (i.e. > disable napi and irqs) and leave the rest to the callers. > > If caller of __ibmvnic_open() is ibmvnic_open(), it should release the > resources immediately. If the caller is do_reset() or do_hard_reset(), > they will release the resources on the next reset. > > [...] Here is the summary with links: - [net,1/1] ibmvnic: don't release napi in __ibmvnic_open() https://git.kernel.org/netdev/net/c/61772b0908c6 You are awesome, thank you!
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index f5d1ba9bea48..50d2e48274eb 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -110,6 +110,7 @@ static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter *adapter, struct ibmvnic_sub_crq_queue *tx_scrq); static void free_long_term_buff(struct ibmvnic_adapter *adapter, struct ibmvnic_long_term_buff *ltb); +static void ibmvnic_disable_irqs(struct ibmvnic_adapter *adapter); struct ibmvnic_stat { char name[ETH_GSTRING_LEN]; @@ -1424,7 +1425,7 @@ static int __ibmvnic_open(struct net_device *netdev) rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP); if (rc) { ibmvnic_napi_disable(adapter); - release_resources(adapter); + ibmvnic_disable_irqs(adapter); return rc; } @@ -1474,9 +1475,6 @@ static int ibmvnic_open(struct net_device *netdev) rc = init_resources(adapter); if (rc) { netdev_err(netdev, "failed to initialize resources\n"); - release_resources(adapter); - release_rx_pools(adapter); - release_tx_pools(adapter); goto out; } } @@ -1493,6 +1491,13 @@ static int ibmvnic_open(struct net_device *netdev) adapter->state = VNIC_OPEN; rc = 0; } + + if (rc) { + release_resources(adapter); + release_rx_pools(adapter); + release_tx_pools(adapter); + } + return rc; }
If __ibmvnic_open() encounters an error such as when setting link state, it calls release_resources() which frees the napi structures needlessly. Instead, have __ibmvnic_open() only clean up the work it did so far (i.e. disable napi and irqs) and leave the rest to the callers. If caller of __ibmvnic_open() is ibmvnic_open(), it should release the resources immediately. If the caller is do_reset() or do_hard_reset(), they will release the resources on the next reset. This fixes following crash that occured when running the drmgr command several times to add/remove a vnic interface: [102056] ibmvnic 30000003 env3: Disabling rx_scrq[6] irq [102056] ibmvnic 30000003 env3: Disabling rx_scrq[7] irq [102056] ibmvnic 30000003 env3: Replenished 8 pools Kernel attempted to read user page (10) - exploit attempt? (uid: 0) BUG: Kernel NULL pointer dereference on read at 0x00000010 Faulting instruction address: 0xc000000000a3c840 Oops: Kernel access of bad area, sig: 11 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries ... CPU: 9 PID: 102056 Comm: kworker/9:2 Kdump: loaded Not tainted 5.16.0-rc5-autotest-g6441998e2e37 #1 Workqueue: events_long __ibmvnic_reset [ibmvnic] NIP: c000000000a3c840 LR: c0080000029b5378 CTR: c000000000a3c820 REGS: c0000000548e37e0 TRAP: 0300 Not tainted (5.16.0-rc5-autotest-g6441998e2e37) MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 28248484 XER: 00000004 CFAR: c0080000029bdd24 DAR: 0000000000000010 DSISR: 40000000 IRQMASK: 0 GPR00: c0080000029b55d0 c0000000548e3a80 c0000000028f0200 0000000000000000 ... NIP [c000000000a3c840] napi_enable+0x20/0xc0 LR [c0080000029b5378] __ibmvnic_open+0xf0/0x430 [ibmvnic] Call Trace: [c0000000548e3a80] [0000000000000006] 0x6 (unreliable) [c0000000548e3ab0] [c0080000029b55d0] __ibmvnic_open+0x348/0x430 [ibmvnic] [c0000000548e3b40] [c0080000029bcc28] __ibmvnic_reset+0x500/0xdf0 [ibmvnic] [c0000000548e3c60] [c000000000176228] process_one_work+0x288/0x570 [c0000000548e3d00] [c000000000176588] worker_thread+0x78/0x660 [c0000000548e3da0] [c0000000001822f0] kthread+0x1c0/0x1d0 [c0000000548e3e10] [c00000000000cf64] ret_from_kernel_thread+0x5c/0x64 Instruction dump: 7d2948f8 792307e0 4e800020 60000000 3c4c01eb 384239e0 f821ffd1 39430010 38a0fff6 e92d1100 f9210028 39200000 <e9030010> f9010020 60420000 e9210020 ---[ end trace 5f8033b08fd27706 ]--- Fixes: ed651a10875f ("ibmvnic: Updated reset handling) Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)