Re: [alsa-devel] [PATCH] fix the kernel panic due to wrong use the dev memory API

[Pierre-Louis Bossart]

On 11/5/18 6:58 PM, He, Bo wrote:
> Hi,
> 	I submit the patch based on tag v4.19.

Please don't post on mailing lists.

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Sent: Tuesday, November 6, 2018 1:02 AM
> To: He, Bo <bo.he@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: liam.r.girdwood@xxxxxxxxxxxxxxx; perex@xxxxxxxx; tiwai@xxxxxxxx; Singh, Guneshwor O <guneshwor.o.singh@xxxxxxxxx>; Periyasamy, SriramX <sriramx.periyasamy@xxxxxxxxx>; Kale, Sanyog R <sanyog.r.kale@xxxxxxxxx>; Kesapragada, Pardha Saradhi <pardha.saradhi.kesapragada@xxxxxxxxx>; kuninori.morimoto.gx@xxxxxxxxxxx; guruprasadx.pawse@xxxxxxxxx; Ughreja, Rakesh A <rakesh.a.ughreja@xxxxxxxxx>; Prakash, Divya1 <divya1.prakash@xxxxxxxxx>; Diwakar, Praveen <praveen.diwakar@xxxxxxxxx>; Zhang, Yanmin <yanmin.zhang@xxxxxxxxx>
> Subject: Re: [alsa-devel] [PATCH] fix the kernel panic due to wrong use the dev memory API
>
>
> On 11/5/18 2:29 AM, He, Bo wrote:
> > skl->dais is allocated with devm_kcalloc, can't free with
> > the krealloc. Memory allocated with devm API is automatically freed on
> > driver detach, Like all other devres resources.
> >
> > Refer to drivers/base/devres.c devm_kmalloc for more details.
>
> What code are you looking at?
>
> I see this in the Mark's tree
>
> int skl_platform_register(struct device *dev) {
>   ÂÂÂ int ret;
>   ÂÂÂ struct snd_soc_dai_driver *dais;
>   ÂÂÂ int num_dais = ARRAY_SIZE(skl_platform_dai);
>   ÂÂÂ struct hdac_bus *bus = dev_get_drvdata(dev);
>   ÂÂÂ struct skl *skl = bus_to_skl(bus);
>
>   ÂÂÂ INIT_LIST_HEAD(&skl->ppl_list);
>   ÂÂÂ INIT_LIST_HEAD(&skl->bind_list);
>
>   ÂÂÂ skl->dais = kmemdup(skl_platform_dai, sizeof(skl_platform_dai),
>   ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ GFP_KERNEL);
>   ÂÂÂ if (!skl->dais) {
>   ÂÂÂ ÂÂÂ ret = -ENOMEM;
>   ÂÂÂ ÂÂÂ goto err;
>   ÂÂÂ }
>
>   ÂÂÂ if (!skl->use_tplg_pcm) {
>   ÂÂÂ ÂÂÂ dais = krealloc(skl->dais, sizeof(skl_fe_dai) +
>   ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ sizeof(skl_platform_dai), GFP_KERNEL);
>
>
> No trace of devm as you mention it? I checked the Chrome tree as well and it's not there.
>
> What am I missing?

The code is completely identical in v4.19. skl->dais is allocated with 
kmemdup, which is a kmalloc+memcpy, i just don't understand what you are 
trying to fix.

> > Signed-off-by: he, bo <bo.he@xxxxxxxxx>
> > ---
> >    sound/soc/intel/skylake/skl-pcm.c | 5 ++---
> >    1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/sound/soc/intel/skylake/skl-pcm.c
> > b/sound/soc/intel/skylake/skl-pcm.c
> > index 823e391..928d314 100644
> > --- a/sound/soc/intel/skylake/skl-pcm.c
> > +++ b/sound/soc/intel/skylake/skl-pcm.c
> > @@ -1438,7 +1438,8 @@ int skl_platform_register(struct device *dev)
> >    	}
> >    
> >    	if (!skl->use_tplg_pcm) {
> > -		dais = krealloc(skl->dais, sizeof(skl_fe_dai) +
> > +		devm_kfree(dev, skl->dais);
> > +		dais = devm_kcalloc(dev, skl->dais, sizeof(skl_fe_dai) +
> >    				sizeof(skl_platform_dai), GFP_KERNEL);
> >    		if (!dais) {
> >    			ret = -ENOMEM;
> > @@ -1472,7 +1473,5 @@ int skl_platform_unregister(struct device *dev)
> >    		}
> >    	}
> >    
> > -	kfree(skl->dais);
> > -
> >    	return 0;
> >    }