Re: [PATCH v3 2/4] soc: amlogic: meson-gx-socinfo-sm: Add Amlogic secure-monitor SoC Information driver

[Viacheslav]

14/03/2024 16.31, Krzysztof Kozlowski wrote:
> On 14/03/2024 13:22, Viacheslav wrote:
> > > +
> > > > +	soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr),
> > > > +				    GFP_KERNEL);
> > > > +	if (!soc_dev_attr)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo);
> > > > +
> > > > +	soc_dev_attr->family = "Amlogic Meson";
> > > > +	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x",
> > > > +					   socinfo.v1.major_id,
> > > > +					   socinfo.v1.chip_rev,
> > > > +					   socinfo.v1.pack_id,
> > > > +					   (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver);
> > > > +	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)",
> > > > +					 socinfo_v1_to_soc_id(socinfo),
> > > > +					 socinfo_v1_to_package_id(socinfo));
> > > > +
> > > > +	soc_dev = soc_device_register(soc_dev_attr);
> > > > +
> > > > +
> > > > +	if (IS_ERR(soc_dev)) {
> > > > +		kfree(soc_dev_attr->revision);
> > > > +		kfree_const(soc_dev_attr->soc_id);
> > > > +		kfree(soc_dev_attr);
> > >
> > > That's a double free. This was not tested.
> >
> >
> > Please, describe the problem.
>
> Test your code. What's the point of arguing over it if regular test
> would show this?
>
> > I don't quite understand what the issue is:
> >
> > - kfree() releases memory allocated with kmalloc()
>
> So point me where is kmalloc(). I don't see. I see only devm.

I missed the point that devm_kzalloc is automatically freed. You are 
right. Thanks!


--
Best regards
Viacheslav