On Mon, Oct 03, 2016 at 03:11:29PM -0600, Jason Gunthorpe wrote: > On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: > > > > Sort of, the typical race is broadly > > > > > > CPU0 CPU1 > > > > > > fops->open() > > > securityfs_remove() > > > kref_put(chip) > > > kfree(chip) > > > kref_get(data->chip.kref) > > > > I see. So could this be reproduced by: > > > > 1. Open binary_measurements. > > 2. rmmod tpm_tis > > 3. Read contents of binary_measurements. > > No, but that method shows the bug I pointed out in my email to Nayna > where the fops stuff is not getting a kref on the chip. > > You need to actually race open and securityfs_remove to see the > kref_get() loose its race and then use-after-free.
So you are worried that get_device() might come when the chip is already gone? > > Yeah, I get it. These securityfs files are nasty in a way compared to > > sysfs attributes that they are not connected to the device hierachy. > > Well, it is not so bad, it is just missing the fence on removal that > sysfs has, or the kref tracking that cdev has. Sadly this is a typical > error within the fops stuff, I've seen it in many places. Do you think that this should be fixed above the driver i.e. add fencing to the securityfs code itself? > > Rather than finding a perfect solution in the code I think a better > > angle would be find ways to test and reproduce possible races, which > > you already started in your response. > > That would be very hard to do, racing two calls like that is > quite difficult in any sort of automatic way, AFAIK. I wonder if SPI has similar file to 'remove' that PCI devices have (checking from the documentation later on). > > Right now we basically don't have any good acceptance criteria to make > > any changes to securityfs stuff. Yes, you can do the analysis (and > > should) but human mind is weak sometimes :) > > Since it is unlikely and not caused by our subsystem I'm inclined to > just leave a comment (that we expect securityfs_remove to fence) and > you can send a note to James to see what they think. Agreed. > Jason /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
