* [PATCH 0/2] usb: gadget: Fix two memleaks in error handling
@ 2020-11-16 12:17 Zhang Qilong
2020-11-16 12:17 ` [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc Zhang Qilong
2020-11-16 12:17 ` [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super Zhang Qilong
0 siblings, 2 replies; 9+ messages in thread
From: Zhang Qilong @ 2020-11-16 12:17 UTC (permalink / raw)
To: balbi, gregkh; +Cc: linux-usb
This patch sets fix two memleaks before error returns.
Zhang Qilong (2):
usb: gadget: f_midi: Fix memleak in f_midi_alloc
usb: gadget: Fix memleak in gadgetfs_fill_super
drivers/usb/gadget/function/f_midi.c | 3 +++
drivers/usb/gadget/legacy/inode.c | 3 +++
2 files changed, 6 insertions(+)
--
2.25.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc
2020-11-16 12:17 [PATCH 0/2] usb: gadget: Fix two memleaks in error handling Zhang Qilong
@ 2020-11-16 12:17 ` Zhang Qilong
2020-11-16 12:59 ` Peter Chen
2020-11-16 16:12 ` Sergei Shtylyov
2020-11-16 12:17 ` [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super Zhang Qilong
1 sibling, 2 replies; 9+ messages in thread
From: Zhang Qilong @ 2020-11-16 12:17 UTC (permalink / raw)
To: balbi, gregkh; +Cc: linux-usb
In the error path, if midi is not null, we should to
free the midi->id if necessary to prevent memleak.
Fixes: b85e9de9e818d ("usb: gadget: f_midi: convert to new function interface with backward compatibility")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
---
drivers/usb/gadget/function/f_midi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 85cb15734aa8..596fd7ce56fb 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1345,7 +1345,10 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
setup_fail:
mutex_unlock(&opts->lock);
+ if (midi)
+ kfree(midi->id);
kfree(midi);
+
return ERR_PTR(status);
}
--
2.25.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super
2020-11-16 12:17 [PATCH 0/2] usb: gadget: Fix two memleaks in error handling Zhang Qilong
2020-11-16 12:17 ` [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc Zhang Qilong
@ 2020-11-16 12:17 ` Zhang Qilong
2020-11-16 13:04 ` Peter Chen
2020-11-16 15:06 ` kernel test robot
1 sibling, 2 replies; 9+ messages in thread
From: Zhang Qilong @ 2020-11-16 12:17 UTC (permalink / raw)
To: balbi, gregkh; +Cc: linux-usb
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;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc
2020-11-16 12:17 ` [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc Zhang Qilong
@ 2020-11-16 12:59 ` Peter Chen
2020-11-16 16:12 ` Sergei Shtylyov
1 sibling, 0 replies; 9+ messages in thread
From: Peter Chen @ 2020-11-16 12:59 UTC (permalink / raw)
To: Zhang Qilong; +Cc: balbi, gregkh, linux-usb
On 20-11-16 20:17:09, Zhang Qilong wrote:
> In the error path, if midi is not null, we should to
> free the midi->id if necessary to prevent memleak.
>
> Fixes: b85e9de9e818d ("usb: gadget: f_midi: convert to new function interface with backward compatibility")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 85cb15734aa8..596fd7ce56fb 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1345,7 +1345,10 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
>
> setup_fail:
> mutex_unlock(&opts->lock);
> + if (midi)
> + kfree(midi->id);
> kfree(midi);
> +
> return ERR_PTR(status);
> }
>
It is better to add another goto label for this memory free.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super
2020-11-16 12:17 ` [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super Zhang Qilong
@ 2020-11-16 13:04 ` Peter Chen
2020-11-17 1:44 ` 答复: " zhangqilong
2020-11-16 15:06 ` kernel test robot
1 sibling, 1 reply; 9+ messages in thread
From: Peter Chen @ 2020-11-16 13:04 UTC (permalink / raw)
To: Zhang Qilong; +Cc: balbi, gregkh, linux-usb
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.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super
2020-11-16 12:17 ` [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super Zhang Qilong
@ 2020-11-16 15:06 ` kernel test robot
2020-11-16 15:06 ` kernel test robot
1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-11-16 15:06 UTC (permalink / raw)
To: Zhang Qilong, balbi, gregkh; +Cc: kbuild-all, clang-built-linux, linux-usb
[-- Attachment #1: Type: text/plain, Size: 4000 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33790 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super
@ 2020-11-16 15:06 ` kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-11-16 15:06 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4106 bytes --]
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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33790 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc
2020-11-16 12:17 ` [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc Zhang Qilong
2020-11-16 12:59 ` Peter Chen
@ 2020-11-16 16:12 ` Sergei Shtylyov
1 sibling, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2020-11-16 16:12 UTC (permalink / raw)
To: Zhang Qilong, balbi, gregkh; +Cc: linux-usb
On 11/16/20 3:17 PM, Zhang Qilong wrote:
> In the error path, if midi is not null, we should to
That "to" not needed here.
> free the midi->id if necessary to prevent memleak.
>
> Fixes: b85e9de9e818d ("usb: gadget: f_midi: convert to new function interface with backward compatibility")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super
2020-11-16 13:04 ` Peter Chen
@ 2020-11-17 1:44 ` zhangqilong
0 siblings, 0 replies; 9+ messages in thread
From: zhangqilong @ 2020-11-17 1:44 UTC (permalink / raw)
To: Peter Chen; +Cc: balbi, gregkh, linux-usb
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-17 1:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 12:17 [PATCH 0/2] usb: gadget: Fix two memleaks in error handling Zhang Qilong
2020-11-16 12:17 ` [PATCH 1/2] usb: gadget: f_midi: Fix memleak in f_midi_alloc Zhang Qilong
2020-11-16 12:59 ` Peter Chen
2020-11-16 16:12 ` Sergei Shtylyov
2020-11-16 12:17 ` [PATCH 2/2] usb: gadget: Fix memleak in gadgetfs_fill_super Zhang Qilong
2020-11-16 13:04 ` Peter Chen
2020-11-17 1:44 ` 答复: " zhangqilong
2020-11-16 15:06 ` kernel test robot
2020-11-16 15:06 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.