On Wed, Feb 5, 2020 at 11:28 AM Dan Carpenter <[email protected]> wrote:
>
> On Wed, Feb 05, 2020 at 11:16:22AM -0800, Dan Williams wrote:
> > Ugh, sorry I thought you were pointing out that there's too many
> > put_device() not the use after free. Yes, the use after free is a bug
> > that needs fixing.
>
> I am complaining about the device_puts... If we call device_put()
> twice then it cause a problem in __nvdimm_create()
>
> drivers/nvdimm/dimm_devs.c
> 506 nvdimm->sec.flags = nvdimm_security_flags(nvdimm,
> NVDIMM_USER);
> 507 nvdimm->sec.ext_flags = nvdimm_security_flags(nvdimm,
> NVDIMM_MASTER);
> 508 nd_device_register(dev);
> 509
> 510 return nvdimm;
> ^^^^^^
> If we call device_put() twice then we this pointer within 4 seconds.
"we this pointer"? We "what" this pointer. 4 seconds is relative to a
runtime test case?
...but yes, point taken this looks like an obvious leak in isolation.
>
> 511 }
>
> The fix is probably to make nd_device_register() return an error code so
> we can do:
>
> ret = nd_device_register(dev);
> if (ret) {
> device_put(&nvdimm->dev);
> return NULL;
> }
>
> return nvdimm;
Ok, so this is meant to be mitigated by the fact that all consumers of
nvdimm_create() perform a nvdimm_bus_check_dimm_count() to validate
that device_add() did not fail. The reason for this organization is to
allow nvdimm initialization to happen in parallel.
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]
