Messages in this thread Patch in this message | | | Date | Mon, 23 Mar 2015 21:41:45 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH v9 2/2] uio: Fix uio driver to refcount device |
| |
On Fri, Mar 20, 2015 at 02:54:44PM +0000, Brian Russell wrote: > Protect uio driver from its owner being unplugged while there are open fds. > Embed struct device in struct uio_device, use refcounting on device, free > uio_device on release. > info struct passed in uio_register_device can be freed on unregister, so null > out the field in uio_unregister_device and check accesses.
That's really not protecting anything except heavy-handed problems...
Look at the code:
> @@ -493,7 +499,7 @@ static unsigned int uio_poll(struct file *filep, poll_table *wait) > struct uio_listener *listener = filep->private_data; > struct uio_device *idev = listener->dev; > > - if (!idev->info->irq) > + if (!idev->info || !idev->info->irq) > return -EIO; >
Great, you checked the irq value, but what if it changes the very next line:
> poll_wait(filep, &idev->wait, wait);
Or any other line within this function? Or any other function that you try to check the value for in the beginning...
This really isn't protecting anything "properly", sorry. Either we don't care about it (hint, I don't think we really do), or we need to properly lock things and check, and protect, things that way.
Please do the first one, as the reference count should be all that we need to care about here.
Sorry I missed this on the previous review, just realized it now this time around.
thanks, greg k-h
| |