On Fri, Nov 13, 2015 at 04:13:44PM +0100, Vegard Nossum wrote: > spidev_remove() potentially sets spidev->spi to NULL, and this happens > with the spidev->spi_lock taken, but without the device_list_lock mutex, > and spidev_release() only takes the device_list_lock mutex for the > spidev->spi dereference above, so presumably it is possible for a new > caller to set spidev->spi to NULL while spidev_release() is in progress. > I think it would be great if somebody who knows the code could have a > look into this to see if there is indeed a problem, and there could be > some comments added to the code to explain There's definitely a potential race there. > 1) why it is (not?) okay to check spidev->spi without the spinlock there, > 2) why the ->spi_lock is not taken inside the device_list_lock mutex in > spidev_remove(), > 3) what are the potential interactions between spidev_release() and > spidev_remove(), if any, It's not safe to use spidev->spi there without the lock. We're safe otherwise because in order to have dereferenced spi elsewhere we must of neccesity have a users refcount so spidev can't go away. This is all fairly standard really, it comes back to the issue Laurent was raising on the kernel summit discussion list about not having helpful infrastructure for managing lifetime with respect to both device removal and files.
Attachment:
signature.asc
Description: PGP signature
