Re: [PATCH] fsi: Aspeed: Fix a potential double free

[Guenter Roeck]

On 1/6/22 10:35 AM, Christophe JAILLET wrote:
> Le 06/01/2022 à 18:25, Guenter Roeck a écrit :
> > On 1/6/22 12:14 AM, Dan Carpenter wrote:
> > > On Mon, Dec 27, 2021 at 07:29:07AM +0100, Greg KH wrote:
> > > > On Sun, Dec 26, 2021 at 05:56:02PM +0100, Christophe JAILLET wrote:
> > > > > 'aspeed' is a devm_alloc'ed, so there is no need to free it explicitly or
> > > > > there will be a double free().
> > > >
> > > > A struct device can never be devm_alloced for obvious reasons.  Perhaps
> > > > that is the real problem here?
> > >
> > > I don't understand how "aspeed" is a struct device.
> >
> > -static void aspeed_master_release(struct device *dev)
> > -{
> > -    struct fsi_master_aspeed *aspeed =
> > -        to_fsi_master_aspeed(dev_to_fsi_master(dev));
> > -
> > -    kfree(aspeed);
> > -}
> >
> > So "dev" is embedded in struct fsi_master, and struct fsi_master is embedded
> > in struct fsi_master_aspeed. Since "struct device" is embedded, the data
> > structure embedding it must be released with the release function, as is done
> > here. The problem is indeed that the data structure is allocated with
> > devm_kzalloc(), which as Greg points out must not be devm_ allocated
> > (because its lifetime does not match the lifetime of devm_ allocated
> > memory).
>
> Thanks a lot for the detailed explanation.
> Crystal clear for me now.
>
> Do you want me to send a patch to remove the devm_ or will you?

Sorry, I am way behind with code reviews. I won't have time to submit a patch.

Guenter