[PATCH 1/1] sg: Command completion after remove oops

[PATCH 1/1] sg: Command completion after remove oops

[brking]

A problem exists todayin the sg driver that if an SG_IO request is
outstanding to a device when it is removed from the system. The
system may oops if that command completes later in time.

1. sg_remove gets called
2. sg_remove calls sg_finish_req_req on all pending requests
   This removes the Sg_request's from the headrp list in the Sg_fd
3. The sleeping SG_IO ioctl is woken. It does nothing and returns.
4. The caller closes the fd, which invokes sg_release
5. sg_release calls sg_remove_sfp. It finds no outstanding commands
   since the headrp list is empty, so it calls __sg_remove_sfp,
   which frees the sfp.
6. Now when sg_cmd_done gets called, sg uses upper_private_data in
   the Scsi_Request, which should point to the srp, which has been
   freed, so it points to freed memory.
7. sg then dereferences the srp pointer to get the sfp, and we oops.

The fix is to NULL out the upper_private_data field in this path,
which sg_cmd_done already checks for, which will prevent the oops
from occurring.


cpu 0x1: Vector: 300 (Data Access) at [c00000000fff7aa0]
    pc: d0000000002bbea8: .sg_cmd_done+0x70/0x394 [sg]
    lr: d000000000073304: .scsi_finish_command+0x10c/0x130 [scsi_mod]
    sp: c00000000fff7d20
   msr: 8000000000009032
   dar: 2f70726f63202f78
 dsisr: 40000000
  current = 0xc0000000024589b0
  paca    = 0xc0000000003da800
    pid   = 7, comm = events/1
[c00000000fff7dc0] d000000000073304 .scsi_finish_command+0x10c/0x130 [scsi_mod]
[c00000000fff7e50] d00000000007317c .scsi_softirq+0x140/0x168 [scsi_mod]
[c00000000fff7ef0] c0000000000634dc .__do_softirq+0xa0/0x17c
[c00000000fff7f90] c000000000018430 .call_do_softirq+0x14/0x24
[c00000000ed472e0] c0000000000142e0 .do_softirq+0x74/0x9c
[c00000000ed47370] c000000000013c9c .do_IRQ+0xe8/0x100
[c00000000ed473f0] c00000000000ae34 HardwareInterrupt_entry+0x8/0x54

c00000000003df28 .smp_call_function+0
x100/0x1d0
[c00000000ed47780] c0000000000ba99c .invalidate_bh_lrus+0x30/0x70
[c00000000ed47810] c0000000000b91a0 .invalidate_bdev+0x18/0x3c
[c00000000ed478a0] c0000000000da7b8 .__invalidate_device+0x70/0x94
[c00000000ed47930] c0000000001d40bc .invalidate_partition+0x4c/0x7c
[c00000000ed479c0] c00000000010a944 .del_gendisk+0x48/0x15c
[c00000000ed47a50] d00000000003d55c .sd_remove+0x34/0xe4 [sd_mod]
[c00000000ed47ae0] c0000000001c5d30 .device_release_driver+0x90/0xb4
[c00000000ed47b70] c0000000001c6130 .bus_remove_device+0xb0/0x12c
[c00000000ed47c00] c0000000001c4378 .device_del+0x120/0x198
[c00000000ed47ca0] d00000000007dcdc .scsi_remove_device+0xb4/0x194 [scsi_mod]
[c00000000ed47d30] d0000000000a5864 .ipr_worker_thread+0x1d4/0x27c [ipr]
[c00000000ed47dd0] c0000000000734c4 .worker_thread+0x238/0x2f4
[c00000000ed47ee0] c0000000000796c0 .kthread+0xcc/0x11c
[c00000000ed47f90] c000000000018ad0 .kernel_thread+0x4c/0x6c


Signed-off-by: Brian King <brking@us.ibm.com>
---

 linux-2.6.12-rc4-bjking1/drivers/scsi/sg.c |    2 ++
 1 files changed, 2 insertions(+)

diff -puN drivers/scsi/sg.c~sg_remove_oops drivers/scsi/sg.c
--- linux-2.6.12-rc4/drivers/scsi/sg.c~sg_remove_oops	2005-05-11 17:15:54.000000000 -0500
+++ linux-2.6.12-rc4-bjking1/drivers/scsi/sg.c	2005-05-11 17:20:30.000000000 -0500
@@ -2472,6 +2472,8 @@ sg_remove_request(Sg_fd * sfp, Sg_reques
 	if ((!sfp) || (!srp) || (!sfp->headrp))
 		return res;
 	write_lock_irqsave(&sfp->rq_list_lock, iflags);
+	if (srp->my_cmdp)
+		srp->my_cmdp->upper_private_data = NULL;
 	prev_rp = sfp->headrp;
 	if (srp == prev_rp) {
 		sfp->headrp = prev_rp->nextrp;
_

Re: [PATCH 1/1] sg: Command completion after remove oops

[Douglas Gilbert]

brking@us.ibm.com wrote:
> A problem exists todayin the sg driver that if an SG_IO request is
> outstanding to a device when it is removed from the system. The
> system may oops if that command completes later in time.
> 
> 1. sg_remove gets called
> 2. sg_remove calls sg_finish_req_req on all pending requests
>    This removes the Sg_request's from the headrp list in the Sg_fd
> 3. The sleeping SG_IO ioctl is woken. It does nothing and returns.
> 4. The caller closes the fd, which invokes sg_release
> 5. sg_release calls sg_remove_sfp. It finds no outstanding commands
>    since the headrp list is empty, so it calls __sg_remove_sfp,
>    which frees the sfp.
> 6. Now when sg_cmd_done gets called, sg uses upper_private_data in
>    the Scsi_Request, which should point to the srp, which has been
>    freed, so it points to freed memory.
> 7. sg then dereferences the srp pointer to get the sfp, and we oops.
> 
> The fix is to NULL out the upper_private_data field in this path,
> which sg_cmd_done already checks for, which will prevent the oops
> from occurring.
> 
> 
> cpu 0x1: Vector: 300 (Data Access) at [c00000000fff7aa0]
>     pc: d0000000002bbea8: .sg_cmd_done+0x70/0x394 [sg]
>     lr: d000000000073304: .scsi_finish_command+0x10c/0x130 [scsi_mod]
>     sp: c00000000fff7d20
>    msr: 8000000000009032
>    dar: 2f70726f63202f78
>  dsisr: 40000000
>   current = 0xc0000000024589b0
>   paca    = 0xc0000000003da800
>     pid   = 7, comm = events/1
> [c00000000fff7dc0] d000000000073304 .scsi_finish_command+0x10c/0x130 [scsi_mod]
> [c00000000fff7e50] d00000000007317c .scsi_softirq+0x140/0x168 [scsi_mod]
> [c00000000fff7ef0] c0000000000634dc .__do_softirq+0xa0/0x17c
> [c00000000fff7f90] c000000000018430 .call_do_softirq+0x14/0x24
> [c00000000ed472e0] c0000000000142e0 .do_softirq+0x74/0x9c
> [c00000000ed47370] c000000000013c9c .do_IRQ+0xe8/0x100
> [c00000000ed473f0] c00000000000ae34 HardwareInterrupt_entry+0x8/0x54
> 
> c00000000003df28 .smp_call_function+0
> x100/0x1d0
> [c00000000ed47780] c0000000000ba99c .invalidate_bh_lrus+0x30/0x70
> [c00000000ed47810] c0000000000b91a0 .invalidate_bdev+0x18/0x3c
> [c00000000ed478a0] c0000000000da7b8 .__invalidate_device+0x70/0x94
> [c00000000ed47930] c0000000001d40bc .invalidate_partition+0x4c/0x7c
> [c00000000ed479c0] c00000000010a944 .del_gendisk+0x48/0x15c
> [c00000000ed47a50] d00000000003d55c .sd_remove+0x34/0xe4 [sd_mod]
> [c00000000ed47ae0] c0000000001c5d30 .device_release_driver+0x90/0xb4
> [c00000000ed47b70] c0000000001c6130 .bus_remove_device+0xb0/0x12c
> [c00000000ed47c00] c0000000001c4378 .device_del+0x120/0x198
> [c00000000ed47ca0] d00000000007dcdc .scsi_remove_device+0xb4/0x194 [scsi_mod]
> [c00000000ed47d30] d0000000000a5864 .ipr_worker_thread+0x1d4/0x27c [ipr]
> [c00000000ed47dd0] c0000000000734c4 .worker_thread+0x238/0x2f4
> [c00000000ed47ee0] c0000000000796c0 .kthread+0xcc/0x11c
> [c00000000ed47f90] c000000000018ad0 .kernel_thread+0x4c/0x6c
> 
> 
> Signed-off-by: Brian King <brking@us.ibm.com>
> ---
> 
>  linux-2.6.12-rc4-bjking1/drivers/scsi/sg.c |    2 ++
>  1 files changed, 2 insertions(+)
> 
> diff -puN drivers/scsi/sg.c~sg_remove_oops drivers/scsi/sg.c
> --- linux-2.6.12-rc4/drivers/scsi/sg.c~sg_remove_oops	2005-05-11 17:15:54.000000000 -0500
> +++ linux-2.6.12-rc4-bjking1/drivers/scsi/sg.c	2005-05-11 17:20:30.000000000 -0500
> @@ -2472,6 +2472,8 @@ sg_remove_request(Sg_fd * sfp, Sg_reques
>  	if ((!sfp) || (!srp) || (!sfp->headrp))
>  		return res;
>  	write_lock_irqsave(&sfp->rq_list_lock, iflags);
> +	if (srp->my_cmdp)
> +		srp->my_cmdp->upper_private_data = NULL;
>  	prev_rp = sfp->headrp;
>  	if (srp == prev_rp) {
>  		sfp->headrp = prev_rp->nextrp;
> _
> 

Brian,
Thanks. This looks correct. James, please apply.

Signed-off-by: Douglas Gilbert <dougg@torque.net>

Doug Gilbert