| Message ID | 20220316031521.422488-1-damien.lemoal@opensource.wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: mpt3sas: fix use after free in _scsih_expander_node_remove() | expand |
On 3/16/22 12:15, Damien Le Moal wrote: > The function mpt3sas_transport_port_remove() called in > _scsih_expander_node_remove() frees the port field of the sas_expander > structure, leading to the following use-after-free spat from Kasan when > the ioc_info() call following that function is executed (e.g. when doing > rmmod of the driver module): > > [ 3479.371167] ================================================================== > [ 3479.378496] BUG: KASAN: use-after-free in _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > [ 3479.386936] Read of size 1 at addr ffff8881c037691c by task rmmod/1531 > [ 3479.393524] > [ 3479.395035] CPU: 18 PID: 1531 Comm: rmmod Not tainted 5.17.0-rc8+ #1436 > [ 3479.401712] Hardware name: Supermicro Super Server/H12SSL-NT, BIOS 2.1 06/02/2021 > [ 3479.409263] Call Trace: > [ 3479.411743] <TASK> > [ 3479.413875] dump_stack_lvl+0x45/0x59 > [ 3479.417582] print_address_description.constprop.0+0x1f/0x120 > [ 3479.423389] ? _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > [ 3479.429469] kasan_report.cold+0x83/0xdf > [ 3479.433438] ? _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > [ 3479.439514] _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > [ 3479.445411] ? _raw_spin_unlock_irqrestore+0x2d/0x40 > [ 3479.452032] scsih_remove+0x525/0xc90 [mpt3sas] > [ 3479.458212] ? mpt3sas_expander_remove+0x1d0/0x1d0 [mpt3sas] > [ 3479.465529] ? down_write+0xde/0x150 > [ 3479.470746] ? up_write+0x14d/0x460 > [ 3479.475840] ? kernfs_find_ns+0x137/0x310 > [ 3479.481438] pci_device_remove+0x65/0x110 > [ 3479.487013] __device_release_driver+0x316/0x680 > [ 3479.493180] driver_detach+0x1ec/0x2d0 > [ 3479.498499] bus_remove_driver+0xe7/0x2d0 > [ 3479.504081] pci_unregister_driver+0x26/0x250 > [ 3479.510033] _mpt3sas_exit+0x2b/0x6cf [mpt3sas] > [ 3479.516144] __x64_sys_delete_module+0x2fd/0x510 > [ 3479.522315] ? free_module+0xaa0/0xaa0 > [ 3479.527593] ? __cond_resched+0x1c/0x90 > [ 3479.532951] ? lockdep_hardirqs_on_prepare+0x273/0x3e0 > [ 3479.539607] ? syscall_enter_from_user_mode+0x21/0x70 > [ 3479.546161] ? trace_hardirqs_on+0x1c/0x110 > [ 3479.551828] do_syscall_64+0x35/0x80 > [ 3479.556884] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 3479.563402] RIP: 0033:0x7f1fc482483b > ... > [ 3479.943087] ================================================================== > > Fix this by reversing the calls to ioc_info() and > mpt3sas_transport_port_remove(). > > Fixes: 7d310f241001 ("scsi: mpt3sas: Get device objects using sas_address & portID") > Cc: stable@vger.kernel.org > Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>> --- > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > index 00792767c620..a3a898262f2d 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > @@ -11055,15 +11055,15 @@ _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc, > mpt3sas_port->hba_port); > } > > - mpt3sas_transport_port_remove(ioc, sas_expander->sas_address, > - sas_expander->sas_address_parent, sas_expander->port); > - > ioc_info(ioc, > "expander_remove: handle(0x%04x), sas_addr(0x%016llx), port:%d\n", > sas_expander->handle, (unsigned long long) > sas_expander->sas_address, > sas_expander->port->port_id); > > + mpt3sas_transport_port_remove(ioc, sas_expander->sas_address, > + sas_expander->sas_address_parent, sas_expander->port); > + > spin_lock_irqsave(&ioc->sas_node_lock, flags); > list_del(&sas_expander->list); > spin_unlock_irqrestore(&ioc->sas_node_lock, flags); Broadcom team, Please review !
On Fri, Mar 18, 2022 at 11:21 AM Damien Le Moal <damien.lemoal@opensource.wdc.com> wrote: > > On 3/16/22 12:15, Damien Le Moal wrote: > > The function mpt3sas_transport_port_remove() called in > > _scsih_expander_node_remove() frees the port field of the sas_expander > > structure, leading to the following use-after-free spat from Kasan when > > the ioc_info() call following that function is executed (e.g. when doing > > rmmod of the driver module): > > > > [ 3479.371167] ================================================================== > > [ 3479.378496] BUG: KASAN: use-after-free in _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > > [ 3479.386936] Read of size 1 at addr ffff8881c037691c by task rmmod/1531 > > [ 3479.393524] > > [ 3479.395035] CPU: 18 PID: 1531 Comm: rmmod Not tainted 5.17.0-rc8+ #1436 > > [ 3479.401712] Hardware name: Supermicro Super Server/H12SSL-NT, BIOS 2.1 06/02/2021 > > [ 3479.409263] Call Trace: > > [ 3479.411743] <TASK> > > [ 3479.413875] dump_stack_lvl+0x45/0x59 > > [ 3479.417582] print_address_description.constprop.0+0x1f/0x120 > > [ 3479.423389] ? _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > > [ 3479.429469] kasan_report.cold+0x83/0xdf > > [ 3479.433438] ? _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > > [ 3479.439514] _scsih_expander_node_remove+0x710/0x750 [mpt3sas] > > [ 3479.445411] ? _raw_spin_unlock_irqrestore+0x2d/0x40 > > [ 3479.452032] scsih_remove+0x525/0xc90 [mpt3sas] > > [ 3479.458212] ? mpt3sas_expander_remove+0x1d0/0x1d0 [mpt3sas] > > [ 3479.465529] ? down_write+0xde/0x150 > > [ 3479.470746] ? up_write+0x14d/0x460 > > [ 3479.475840] ? kernfs_find_ns+0x137/0x310 > > [ 3479.481438] pci_device_remove+0x65/0x110 > > [ 3479.487013] __device_release_driver+0x316/0x680 > > [ 3479.493180] driver_detach+0x1ec/0x2d0 > > [ 3479.498499] bus_remove_driver+0xe7/0x2d0 > > [ 3479.504081] pci_unregister_driver+0x26/0x250 > > [ 3479.510033] _mpt3sas_exit+0x2b/0x6cf [mpt3sas] > > [ 3479.516144] __x64_sys_delete_module+0x2fd/0x510 > > [ 3479.522315] ? free_module+0xaa0/0xaa0 > > [ 3479.527593] ? __cond_resched+0x1c/0x90 > > [ 3479.532951] ? lockdep_hardirqs_on_prepare+0x273/0x3e0 > > [ 3479.539607] ? syscall_enter_from_user_mode+0x21/0x70 > > [ 3479.546161] ? trace_hardirqs_on+0x1c/0x110 > > [ 3479.551828] do_syscall_64+0x35/0x80 > > [ 3479.556884] entry_SYSCALL_64_after_hwframe+0x44/0xae > > [ 3479.563402] RIP: 0033:0x7f1fc482483b > > ... > > [ 3479.943087] ================================================================== > > > > Fix this by reversing the calls to ioc_info() and > > mpt3sas_transport_port_remove(). > > > > Fixes: 7d310f241001 ("scsi: mpt3sas: Get device objects using sas_address & portID") > > Cc: stable@vger.kernel.org > > Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>> --- > > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > > index 00792767c620..a3a898262f2d 100644 > > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c > > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > > @@ -11055,15 +11055,15 @@ _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc, > > mpt3sas_port->hba_port); > > } > > > > - mpt3sas_transport_port_remove(ioc, sas_expander->sas_address, > > - sas_expander->sas_address_parent, sas_expander->port); > > - > > ioc_info(ioc, > > "expander_remove: handle(0x%04x), sas_addr(0x%016llx), port:%d\n", > > sas_expander->handle, (unsigned long long) > > sas_expander->sas_address, > > sas_expander->port->port_id); > > > > + mpt3sas_transport_port_remove(ioc, sas_expander->sas_address, > > + sas_expander->sas_address_parent, sas_expander->port); > > + Damien, Can we save the port_id to a local variable and use it while printking the "expander_remove" message? In older issue logs, in some cases we have observed that device removal at SML get's stuck. So, it will be easy to identify/debug whether device removal got stuck at the SML or not if we have a "expander_remove" print message after calling the mpt3sas_transport_port_remove() (i.e. after device is successfully unregistered at the SML). Thanks, Sreekanth > > spin_lock_irqsave(&ioc->sas_node_lock, flags); > > list_del(&sas_expander->list); > > spin_unlock_irqrestore(&ioc->sas_node_lock, flags); > > > Broadcom team, > > Please review ! > > > -- > Damien Le Moal > Western Digital Research
On 2022/03/20 22:29, Sreekanth Reddy wrote: > Damien, > > Can we save the port_id to a local variable and use it while printking > the "expander_remove" message? In older issue logs, in some cases we > have observed that device removal at SML get's stuck. So, it will be > easy to identify/debug whether device removal got stuck at the SML or > not if we have a "expander_remove" print message after calling the > mpt3sas_transport_port_remove() (i.e. after device is successfully > unregistered at the SML). OK. I will send a V2.
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index 00792767c620..a3a898262f2d 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -11055,15 +11055,15 @@ _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc, mpt3sas_port->hba_port); } - mpt3sas_transport_port_remove(ioc, sas_expander->sas_address, - sas_expander->sas_address_parent, sas_expander->port); - ioc_info(ioc, "expander_remove: handle(0x%04x), sas_addr(0x%016llx), port:%d\n", sas_expander->handle, (unsigned long long) sas_expander->sas_address, sas_expander->port->port_id); + mpt3sas_transport_port_remove(ioc, sas_expander->sas_address, + sas_expander->sas_address_parent, sas_expander->port); + spin_lock_irqsave(&ioc->sas_node_lock, flags); list_del(&sas_expander->list); spin_unlock_irqrestore(&ioc->sas_node_lock, flags);
The function mpt3sas_transport_port_remove() called in _scsih_expander_node_remove() frees the port field of the sas_expander structure, leading to the following use-after-free spat from Kasan when the ioc_info() call following that function is executed (e.g. when doing rmmod of the driver module): [ 3479.371167] ================================================================== [ 3479.378496] BUG: KASAN: use-after-free in _scsih_expander_node_remove+0x710/0x750 [mpt3sas] [ 3479.386936] Read of size 1 at addr ffff8881c037691c by task rmmod/1531 [ 3479.393524] [ 3479.395035] CPU: 18 PID: 1531 Comm: rmmod Not tainted 5.17.0-rc8+ #1436 [ 3479.401712] Hardware name: Supermicro Super Server/H12SSL-NT, BIOS 2.1 06/02/2021 [ 3479.409263] Call Trace: [ 3479.411743] <TASK> [ 3479.413875] dump_stack_lvl+0x45/0x59 [ 3479.417582] print_address_description.constprop.0+0x1f/0x120 [ 3479.423389] ? _scsih_expander_node_remove+0x710/0x750 [mpt3sas] [ 3479.429469] kasan_report.cold+0x83/0xdf [ 3479.433438] ? _scsih_expander_node_remove+0x710/0x750 [mpt3sas] [ 3479.439514] _scsih_expander_node_remove+0x710/0x750 [mpt3sas] [ 3479.445411] ? _raw_spin_unlock_irqrestore+0x2d/0x40 [ 3479.452032] scsih_remove+0x525/0xc90 [mpt3sas] [ 3479.458212] ? mpt3sas_expander_remove+0x1d0/0x1d0 [mpt3sas] [ 3479.465529] ? down_write+0xde/0x150 [ 3479.470746] ? up_write+0x14d/0x460 [ 3479.475840] ? kernfs_find_ns+0x137/0x310 [ 3479.481438] pci_device_remove+0x65/0x110 [ 3479.487013] __device_release_driver+0x316/0x680 [ 3479.493180] driver_detach+0x1ec/0x2d0 [ 3479.498499] bus_remove_driver+0xe7/0x2d0 [ 3479.504081] pci_unregister_driver+0x26/0x250 [ 3479.510033] _mpt3sas_exit+0x2b/0x6cf [mpt3sas] [ 3479.516144] __x64_sys_delete_module+0x2fd/0x510 [ 3479.522315] ? free_module+0xaa0/0xaa0 [ 3479.527593] ? __cond_resched+0x1c/0x90 [ 3479.532951] ? lockdep_hardirqs_on_prepare+0x273/0x3e0 [ 3479.539607] ? syscall_enter_from_user_mode+0x21/0x70 [ 3479.546161] ? trace_hardirqs_on+0x1c/0x110 [ 3479.551828] do_syscall_64+0x35/0x80 [ 3479.556884] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 3479.563402] RIP: 0033:0x7f1fc482483b ... [ 3479.943087] ================================================================== Fix this by reversing the calls to ioc_info() and mpt3sas_transport_port_remove(). Fixes: 7d310f241001 ("scsi: mpt3sas: Get device objects using sas_address & portID") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> --- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)