On 2019-03-21 7:33 p.m., Bart Van Assche wrote:
On Thu, 2019-03-21 at 19:11 -0400, Douglas Gilbert wrote:
That doesn't sound right. If it was correct then sg_open() and sg_release()
have mutex overkill (and I don't think that is caused by the complexity of
adding O_EXCL which is damn hard to implement correctly).
Example with existing ch driver code, two threads T1 and T2:
T1 T2
========================================
f1 = open("/dev/ch1")
....
close(f1) f2 = open("dev/sg1")
So if f2=open() gets ch (a pointer) but _before_ it does kref_get(),
close(f1) gets in and does kref_put(&ch->ref, ch_destroy), ref goes
to 0 and ch_destroy() gets called and now ch is dangling ....
Hi Doug,
I don't think that what you described can happen. The kref_put() call in
ch_release() can only drop the final reference after ch_remove() has been
called. Before ch_remove() calls kref_put() it removes the index from the
idr so ch_open() won't find that index in the idr anymore. In other words,
ch_open() can never encounter a zero refcount for an index that it found
in the idr.
Well its another "no file scope" char driver. Open() twice on the same sch
device, close() once and
static int
ch_release(struct inode *inode, struct file *file)
{
scsi_changer *ch = file->private_data;
scsi_device_put(ch->device);
ch->device = NULL; // <<===== WTF?
file->private_data = NULL;
kref_put(&ch->ref, ch_destroy);
return 0;
}
So the next SCSI command sent on the remaining file descriptor should oops
due to the line shown above. ch->device is set in the ch_probe() function!
And the next open() on that device will oops due to scsi_device_get(NULL) ...
Doug Gilbert
