| Message ID | 20201116121710.1546690-3-zhangqilong3@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: Fix two memleaks in error handling | expand |
On 20-11-16 20:17:10, Zhang Qilong wrote: > usb_get_gadget_udc_name will alloc memory for CHIP > in "Enomem" branch. we should free it before error > returns to prevent memleak. > > Fixes: 175f712119c57 ("usb: gadget: provide interface for legacy gadgets to get UDC name") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> > --- > drivers/usb/gadget/legacy/inode.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c > index 1b430b36d0a6..3cefc27be098 100644 > --- a/drivers/usb/gadget/legacy/inode.c > +++ b/drivers/usb/gadget/legacy/inode.c > @@ -2039,6 +2039,9 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc) > return 0; > > Enomem: > + free(CHIP); > + CHIP = NULL; > + > return -ENOMEM; > } It seems the dev_new also allocate the memory, would you please change it too, also, create a new goto entry for it.
Hi Zhang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on balbi-usb/testing/next]
[also build test ERROR on usb/usb-testing linus/master peter.chen-usb/ci-for-usb-next v5.10-rc4 next-20201116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Zhang-Qilong/usb-gadget-Fix-two-memleaks-in-error-handling/20201116-201612
base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: powerpc-randconfig-r015-20201116 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c044709b8fbea2a9a375e4173a6bd735f6866c0c)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/0day-ci/linux/commit/0dfab8598cb8b814fc17011d7a15ff463840e1dd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Zhang-Qilong/usb-gadget-Fix-two-memleaks-in-error-handling/20201116-201612
git checkout 0dfab8598cb8b814fc17011d7a15ff463840e1dd
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/usb/gadget/legacy/inode.c:2042:2: error: implicitly declaring library function 'free' with type 'void (void *)' [-Werror,-Wimplicit-function-declaration]
free(CHIP);
^
drivers/usb/gadget/legacy/inode.c:2042:2: note: include the header <stdlib.h> or explicitly provide a declaration for 'free'
>> drivers/usb/gadget/legacy/inode.c:2042:7: error: passing 'const char *' to parameter of type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
free(CHIP);
^~~~
2 errors generated.
vim +2042 drivers/usb/gadget/legacy/inode.c
1990
1991 static int
1992 gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
1993 {
1994 struct inode *inode;
1995 struct dev_data *dev;
1996
1997 if (the_device)
1998 return -ESRCH;
1999
2000 CHIP = usb_get_gadget_udc_name();
2001 if (!CHIP)
2002 return -ENODEV;
2003
2004 /* superblock */
2005 sb->s_blocksize = PAGE_SIZE;
2006 sb->s_blocksize_bits = PAGE_SHIFT;
2007 sb->s_magic = GADGETFS_MAGIC;
2008 sb->s_op = &gadget_fs_operations;
2009 sb->s_time_gran = 1;
2010
2011 /* root inode */
2012 inode = gadgetfs_make_inode (sb,
2013 NULL, &simple_dir_operations,
2014 S_IFDIR | S_IRUGO | S_IXUGO);
2015 if (!inode)
2016 goto Enomem;
2017 inode->i_op = &simple_dir_inode_operations;
2018 if (!(sb->s_root = d_make_root (inode)))
2019 goto Enomem;
2020
2021 /* the ep0 file is named after the controller we expect;
2022 * user mode code can use it for sanity checks, like we do.
2023 */
2024 dev = dev_new ();
2025 if (!dev)
2026 goto Enomem;
2027
2028 dev->sb = sb;
2029 dev->dentry = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
2030 if (!dev->dentry) {
2031 put_dev(dev);
2032 goto Enomem;
2033 }
2034
2035 /* other endpoint files are available after hardware setup,
2036 * from binding to a controller.
2037 */
2038 the_device = dev;
2039 return 0;
2040
2041 Enomem:
> 2042 free(CHIP);
2043 CHIP = NULL;
2044
2045 return -ENOMEM;
2046 }
2047
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> > On 20-11-16 20:17:10, Zhang Qilong wrote: > > usb_get_gadget_udc_name will alloc memory for CHIP in "Enomem" branch. > > we should free it before error returns to prevent memleak. > > > > Fixes: 175f712119c57 ("usb: gadget: provide interface for legacy > > gadgets to get UDC name") > > Reported-by: Hulk Robot <hulkci@huawei.com> > > Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> > > --- > > drivers/usb/gadget/legacy/inode.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/usb/gadget/legacy/inode.c > > b/drivers/usb/gadget/legacy/inode.c > > index 1b430b36d0a6..3cefc27be098 100644 > > --- a/drivers/usb/gadget/legacy/inode.c > > +++ b/drivers/usb/gadget/legacy/inode.c > > @@ -2039,6 +2039,9 @@ gadgetfs_fill_super (struct super_block *sb, struct > fs_context *fc) > > return 0; > > > > Enomem: > > + free(CHIP); > > + CHIP = NULL; > > + > > return -ENOMEM; > > } > > It seems the dev_new also allocate the memory, would you please change it > too, also, create a new goto entry for it. If gadgetfs_create_file fails, the dev_new will be freed in put_dev. Thanks, Zhang > -- > > Thanks, > Peter Chen
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index 1b430b36d0a6..3cefc27be098 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -2039,6 +2039,9 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc) return 0; Enomem: + free(CHIP); + CHIP = NULL; + return -ENOMEM; }
usb_get_gadget_udc_name will alloc memory for CHIP in "Enomem" branch. we should free it before error returns to prevent memleak. Fixes: 175f712119c57 ("usb: gadget: provide interface for legacy gadgets to get UDC name") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> --- drivers/usb/gadget/legacy/inode.c | 3 +++ 1 file changed, 3 insertions(+)