Re: [PATCH v5] dmaengine: idxd: Do not use devm for 'struct device' object allocation

Jason Gunthorpe
2021-03-24T23:36:22Z

On Wed, Mar 24, 2021 at 10:52:52PM +0300, Dan Carpenter wrote:
> On Wed, Mar 24, 2021 at 01:52:46PM -0300, Jason Gunthorpe wrote:
> > On Wed, Mar 24, 2021 at 09:13:35AM -0700, Dan Williams wrote:
> >
> > > Which is just:
> > >
> > > device_initialize()
> > > dev_set_name()
> > >
> > > ...then the name is set as early as the device is ready to filled in
> > > with other details. Just checking for dev_set_name() failures does not
> > > move the api forward in my opinion.
> >
> > This doesn't work either as the release function must be set after
> > initialize but before dev_set_name(), otherwise we both can't and must
> > call put_device() after something like this fails.
> >
> > I can't see an option other than bite the bullet and fix things.
> >
> > A static tool to look for these special lifetime rules around the
> > driver core would be nice.
>
> If y'all are specific enough about what you want, then I can write the
> check for you. What I really want is some buggy sample code and the
> warning you want me to print. I kind of vaguely know that devm_ life
> time rules are tricky but I don't know the details.

This is driver core rules.

The setup is:

struct foo_device
{
struct device dev;
}

struct foo_device *fdev = kzalloc(sizeo(*fdev), GFP_KERNEL);

Then in each of these situations:

device_initialize(&fdev->dev);
// WARNING initialized struct device's must be destroyed with put_device()
kfree(fdev);

And:
dev_set_name(&fdev->dev,..)
// WARNING not using put_device after dev_set_name() leaks memory
kfree(fdev);

And:
device_register(&fdev->dev)
// WARNING not using put_device after device_register() leaks memory
kfree(fdev);

ie kfree is not allowed on any control path after the indicated
function calls.

It is systemically wrong everywhere, here is my first hit in grep:

https://elixir.bootlin.com/linux/latest/source/arch/arm/common/locomo.c#L258

Jason