diff mbox series

[v2] scsi: mpt3sas: fix use after free in _scsih_expander_node_remove()

Message ID 20220322055702.95276-1-damien.lemoal@opensource.wdc.com (mailing list archive)
State Accepted
Headers show
Series [v2] scsi: mpt3sas: fix use after free in _scsih_expander_node_remove() | expand

Commit Message

Damien Le Moal March 22, 2022, 5:57 a.m. UTC
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 splat 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 introducing the local variable port_id to store the port ID
value before executing mpt3sas_transport_port_remove(). This local
variable is then used in the call to ioc_info() instead of dereferencing
the freed port structure.

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>
---
Changes from v1:
* Use local variable to store the port ID instead of reversing the calls
  to ioc_info() and mpt3sas_transport_port_remove().

 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Sreekanth Reddy March 22, 2022, 6:34 a.m. UTC | #1
On Tue, Mar 22, 2022 at 11:28 AM 'Damien Le Moal' via
PDL-MPT-FUSIONLINUX <mpt-fusionlinux.pdl@broadcom.com> 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 splat 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 introducing the local variable port_id to store the port ID
> value before executing mpt3sas_transport_port_remove(). This local
> variable is then used in the call to ioc_info() instead of dereferencing
> the freed port structure.
>
> 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>

Thanks for the patch.
Ack-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>

> ---
> Changes from v1:
> * Use local variable to store the port ID instead of reversing the calls
>   to ioc_info() and mpt3sas_transport_port_remove().
>
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 00792767c620..7e476f50935b 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -11035,6 +11035,7 @@ _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc,
>  {
>         struct _sas_port *mpt3sas_port, *next;
>         unsigned long flags;
> +       int port_id;
>
>         /* remove sibling ports attached to this expander */
>         list_for_each_entry_safe(mpt3sas_port, next,
> @@ -11055,6 +11056,8 @@ _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc,
>                             mpt3sas_port->hba_port);
>         }
>
> +       port_id = sas_expander->port->port_id;
> +
>         mpt3sas_transport_port_remove(ioc, sas_expander->sas_address,
>             sas_expander->sas_address_parent, sas_expander->port);
>
> @@ -11062,7 +11065,7 @@ _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *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);
> +           port_id);
>
>         spin_lock_irqsave(&ioc->sas_node_lock, flags);
>         list_del(&sas_expander->list);
> --
> 2.35.1
>
Martin K. Petersen March 30, 2022, 2:56 a.m. UTC | #2
Damien,

> 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 splat from Kasan
> when the ioc_info() call following that function is executed
> (e.g. when doing rmmod of the driver module):

Applied to 5.18/scsi-staging, thanks!
Martin K. Petersen April 7, 2022, 1:35 p.m. UTC | #3
On Tue, 22 Mar 2022 14:57:02 +0900, 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 splat 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] ==================================================================
> 
> [...]

Applied to 5.18/scsi-fixes, thanks!

[1/1] scsi: mpt3sas: fix use after free in _scsih_expander_node_remove()
      https://git.kernel.org/mkp/scsi/c/87d663d40801
diff mbox series

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 00792767c620..7e476f50935b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -11035,6 +11035,7 @@  _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc,
 {
 	struct _sas_port *mpt3sas_port, *next;
 	unsigned long flags;
+	int port_id;
 
 	/* remove sibling ports attached to this expander */
 	list_for_each_entry_safe(mpt3sas_port, next,
@@ -11055,6 +11056,8 @@  _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *ioc,
 			    mpt3sas_port->hba_port);
 	}
 
+	port_id = sas_expander->port->port_id;
+
 	mpt3sas_transport_port_remove(ioc, sas_expander->sas_address,
 	    sas_expander->sas_address_parent, sas_expander->port);
 
@@ -11062,7 +11065,7 @@  _scsih_expander_node_remove(struct MPT3SAS_ADAPTER *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);
+	    port_id);
 
 	spin_lock_irqsave(&ioc->sas_node_lock, flags);
 	list_del(&sas_expander->list);