linux-kernel - RE: [EXT] Re: [PATCH] USB: gadget: Add ID numbers to configfs-gadget driver names

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <HE1PR0401MB2331E31670508F5BD4B00E1688E19@HE1PR0401MB2331.eurprd04.prod.outlook.com>
Date:   Thu, 15 Dec 2022 16:15:55 +0000
From:   Frank Li <frank.li@....com>
To:     Chanh Nguyen <chanh@...eremail.onmicrosoft.com>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Chanh Nguyen <chanh@...amperecomputing.com>
CC:     OpenBMC Maillist <openbmc@...ts.ozlabs.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dan Vacura <w36195@...orola.com>,
        Jakob Koschel <jakobkoschel@...il.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Vijayavardhan Vennapusa <vvreddy@...eaurora.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Open Source Submission <patches@...erecomputing.com>,
        Rondreis <linhaoguo86@...il.com>
Subject: RE: [EXT] Re: [PATCH] USB: gadget: Add ID numbers to configfs-gadget
 driver names

]
> >
> > Move all free into gadget_info_attr_release(), just before kfree(gi)
> > Driver.name and gi create at the same place,
> > Free should be the same place also.
> >
> 
> Thanks a lot for the quick review comment.
> 
> As per my observation through the test, on the first mount, the
> virtual-media the gadgets_make() is called, then later, when unmount,
> the gadgets_drop() is called and followed by gadget_info_attr_release().
> 
> The gadget_info_attr_release() is registered as .release() of
> gadget_root_type for the gi->group via the call
> "config_group_init_type_name(&gi->group, name, &gadget_root_type);"
> 
> In general, the .release() will be called only for the group. There is
> nothing to guarantee that the group will always be registered, ie:
> incase without the call to "config_group_init_type_name(&gi->group,
> name, &gadget_root_type);"
> 
> In this patch, what is added is an ida number to be used to make up the
> composite driver name. This is done in gadgets_make() so we'd like to
> add the cleanup code to gadgets_drop() as they are registered together
> in the same place and would be a little easier to read than adding them
> to _release() as the code below:
> 
>      static struct configfs_group_operations gadgets_ops = {
>          .make_group = &gadgets_make,
>          .drop_item = &gadgets_drop,
>      };
> 
> Anyway, we still doubt that there might be something that we have missed
> so please let me know the reason why putting cleanup codes to _release()
> would be a better solution.

Let's simply the logic

Gadget_make()
{
        gi=kazalloc();
        gi->composite.gadget_driver.driver.name = kasprintf();
        gi->compositr.gadget_drver.function = kstrdup();
}

this is only place to create gi and driver.name. 
no other place to set driver.name,  so I think gi take ownship of driver.name. 

so we only track when gi free,   just free driver.name before gi free. 

Related malloc and free should be pair together. 

Gadget_info_attr_release()
{
	Kfree(gi->composite.gadget_driver.function)
	Kfree(gi->composiste.gadget_driver.driver.name);
	Kfree(gi);
}

Gadget_driver.driver.name should be similar as gadget_driver.function

If driver.name is freed in gadget_drop(), gi can not actually be reused again.



> 
> Thank you and best regards,
> - Chanh
> 
> >
> >>>>        config_item_put(item);
> >>>>    }
> >>>