Skip to content

sg: fix races during device removal (v2)

Tony Battersby linux-scsi 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest2 patches 35 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2sg: fix races during device removal (v4)drivers/scsi/sg.c+186 -2001 reply
1/2sg: fix races during device removal (v5)drivers/scsi/sg.c+201 -2192 replies
1/2sg: fix races during device removal (v6)drivers/scsi/sg.c+201 -2174 replies
2/2sg: fix races with ioctl(SG_IO)drivers/scsi/sg.c+12 -90 replies
2/2sg: fix races with ioctl(SG_IO) (v2)drivers/scsi/sg.c+14 -251 reply

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/2] sg: fix races during device removal (v2)

This is version 2 of my patch, this time using a kref instead of int. There are also a lot of other changes since the first version since I found even more bugs both through testing and source code analysis. I have split the patch up into two parts: the first patch fixes races between open, close, device removal, and command completion. The second patch fixes some races I spotted in ioctl(SG_IO).

Below are a list of test cases fixed by the patch.

----------

test #1

open /dev/sgX
send a command that takes a long time (e.g. any tape drive seek command)
before command completes, echo 1 > /sys/class/scsi_generic/sgX/device/delete

without patch:
oops

with patch:
test program gets ENODEV immediately
keventd sleeps until the cmd is complete
this is suboptimal since it starves other users of keventd while waiting for the command to complete, but it is better than an oops

----------

test #2

open /dev/sgX
send a command that takes a long time (e.g. any tape drive seek command) without waiting for it to complete
close fd
before command completes, echo 1 > /sys/class/scsi_generic/sgX/device/delete

without patch:
oops when the command does complete (sg_rq_end_io() bad pointer deref)

with patch:
keventd sleeps until the cmd is complete
this is suboptimal since it starves other users of keventd while waiting for the command to complete, but it is better than an oops

----------

test #3

open /dev/sgX
send a command that takes a long time (e.g. any tape drive seek command) without waiting for it to complete
close fd
rmmod sg

without patch:
rmmod succeeds without waiting for the command to complete oops when the command does complete (sg_rq_end_io() callback no longer exists)

with patch:
sg module usage count does not drop to 0 until the command completes, so cannot rmmod

----------

test #4

open /dev/sgX
loop: send commands, check results
unplug SAS cable
mptsas automatically removes the device and fails active commands the test program detects the failed commands and closes its fds at the same time
this results in the following sequence:
sg_remove() enter
sg_release() enter
sg_release() exit
sg_remove() exit

without patch:
oops

with patch:
ok

----------

test #5

open /dev/sgX
loop: send commands, check results
unplug SAS cable
mptsas automatically removes the device and fails active commands the test program detects the failed commands, but sleeps for a second before closing its fds
this results in the following sequence:
sg_remove() enter
sg_remove() exit
sg_release() enter
sg_release() exit

without patch:
oops

with patch:
ok