Re: [PATCH] fix sd open/remove race

[James Bottomley <James.Bottomley@steeleye.com>]

On Fri, 2004-04-09 at 09:11, Alan Stern wrote:
> The patch is almost but not quite correct.  It acquires the new semaphore 
> unnecessarily during scsi_disk_put, it fails to acquire it when erasing 
> the gendisk private_data, and it needs to erase the private_data during 
> sd_remove (not scsi_disk_release).  It also contains a syntax error.

Heh, the additional semicolon I'll remove.

However, the rest of your patch is wrong.

The reason is that what you're trying to synchronise is the naming
lookup (the gendisk open).  You have to allow currently open devices to
continue to operate.  NULL'ing out the disk->private_data before
del_gendisk would mean it was null for sd_release, so every attempt to
close the device after removal would oops.  That's why the NULL must be
done in the reference release routine, not in the disconnection
indication routine.  The race you're trying to prevent is the 0->1
reference after disconnection.

The original patch, By the way, did hold the sd_ref_sem around the NULL
of disk->private_data because the scsi_disk_release is triggered from
kobject_put on last put (and thus is under the semaphore).

James