On 21/02/17 12:09 PM, Jason Gunthorpe wrote: > On Mon, Feb 20, 2017 at 10:00:46PM -0700, Logan Gunthorpe wrote: >> This patch updates core/ucm.c which didn't originally use the >> cdev.kobj.parent with it's parent device. I did not look heavily into >> whether this was a bug or not, but it seems likely to me there would >> be a use before free. > > Hum, is probably safe - ib_ucm_remove_one can only happen on module > unload and the cdev holds the module lock while open.
Ah, yes looks like you are correct. > Even so device_add_cdev should be used anyhow. Agreed. >> I also took a look at core/uverbs_main.c, core/user_mad.c and >> hw/hfi1/device.c which utilize cdev.kobj.parent but because the >> infiniband core seems to use kobjs internally instead of struct devices >> they could not be converted to use the new helper API and still >> directly manipulate the internals of the kobj. > > Yes, and hfi1 had the same use-afte-free bug when it was first > submitted as well. IHMO cdev should have a helper entry for this style > of use case as well. I agree, but I'm not sure what that api should look like. Same thing but kobject_add_cdev??? >> diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c >> index e0a995b..38ea316 100644 >> +++ b/drivers/infiniband/core/ucm.c >> @@ -1283,18 +1283,20 @@ static void ib_ucm_add_one(struct ib_device *device) >> set_bit(devnum, dev_map); >> } >> >> + device_initialize(&ucm_dev->dev); > > Ah, this needs more help. Once device_initialize is called put_device > should be the error-unwind, can you use something more like this? Is that true? Once device_register or device_add is called then you need to use put_device. But I didn't think that's true for device_initialize. In fact device_add is what does the first get_device so this doesn't add up to me. device_initialize only inits some structures it doesn't do anything that needs to be torn down -- at least that I'm aware of. I know the DAX code only uses put_device after device_add. [1] Logan [1] http://lxr.free-electrons.com/source/drivers/dax/dax.c#L713 _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm