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. > 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. > 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. > 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. Jason ------------------------------------------------------------------------------ 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
