drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'

drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'

[Dan Carpenter]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   99d99825fc075fd24b60cc9cf0fb1e20b9c16b0f
commit: 914d9d831e6126a6e7a92e27fcfaa250671be42c of: dynamic: Refactor action prints to not use "%pOF" inside devtree_lock
config: x86_64-randconfig-161-20230831 (https://download.01.org/0day-ci/archive/20230901/202309011059.EOdr4im9-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230901/202309011059.EOdr4im9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202309011059.EOdr4im9-lkp@intel.com/

smatch warnings:
drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'

vim +/ce +935 drivers/of/dynamic.c

201c910bd6898d Pantelis Antoniou 2014-07-04  925  int of_changeset_action(struct of_changeset *ocs, unsigned long action,
201c910bd6898d Pantelis Antoniou 2014-07-04  926  		struct device_node *np, struct property *prop)
201c910bd6898d Pantelis Antoniou 2014-07-04  927  {
201c910bd6898d Pantelis Antoniou 2014-07-04  928  	struct of_changeset_entry *ce;
201c910bd6898d Pantelis Antoniou 2014-07-04  929  
201c910bd6898d Pantelis Antoniou 2014-07-04  930  	ce = kzalloc(sizeof(*ce), GFP_KERNEL);
606ad42aa3b1fe Rob Herring       2016-06-15  931  	if (!ce)
201c910bd6898d Pantelis Antoniou 2014-07-04  932  		return -ENOMEM;
606ad42aa3b1fe Rob Herring       2016-06-15  933  
914d9d831e6126 Rob Herring       2023-08-18  934  	if (WARN_ON(action >= ARRAY_SIZE(action_names)))
914d9d831e6126 Rob Herring       2023-08-18 @935  		return -EINVAL;

No kfree(ce).  Probably we move this check before the kmalloc()?

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'

[Geert Uytterhoeven]

Hi Dan,

On Thu, Sep 7, 2023 at 12:52 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   99d99825fc075fd24b60cc9cf0fb1e20b9c16b0f
> commit: 914d9d831e6126a6e7a92e27fcfaa250671be42c of: dynamic: Refactor action prints to not use "%pOF" inside devtree_lock
> config: x86_64-randconfig-161-20230831 (https://download.01.org/0day-ci/archive/20230901/202309011059.EOdr4im9-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
> reproduce: (https://download.01.org/0day-ci/archive/20230901/202309011059.EOdr4im9-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202309011059.EOdr4im9-lkp@intel.com/
>
> smatch warnings:
> drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'
>
> vim +/ce +935 drivers/of/dynamic.c
>
> 201c910bd6898d Pantelis Antoniou 2014-07-04  925  int of_changeset_action(struct of_changeset *ocs, unsigned long action,
> 201c910bd6898d Pantelis Antoniou 2014-07-04  926                struct device_node *np, struct property *prop)
> 201c910bd6898d Pantelis Antoniou 2014-07-04  927  {
> 201c910bd6898d Pantelis Antoniou 2014-07-04  928        struct of_changeset_entry *ce;
> 201c910bd6898d Pantelis Antoniou 2014-07-04  929
> 201c910bd6898d Pantelis Antoniou 2014-07-04  930        ce = kzalloc(sizeof(*ce), GFP_KERNEL);
> 606ad42aa3b1fe Rob Herring       2016-06-15  931        if (!ce)
> 201c910bd6898d Pantelis Antoniou 2014-07-04  932                return -ENOMEM;
> 606ad42aa3b1fe Rob Herring       2016-06-15  933
> 914d9d831e6126 Rob Herring       2023-08-18  934        if (WARN_ON(action >= ARRAY_SIZE(action_names)))
> 914d9d831e6126 Rob Herring       2023-08-18 @935                return -EINVAL;
>
> No kfree(ce).  Probably we move this check before the kmalloc()?

Yes, moving up sounds great.
Care to send a patch?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'

[Dan Carpenter]

On Thu, Sep 07, 2023 at 01:30:09PM +0200, Geert Uytterhoeven wrote:
> Hi Dan,
> 
> On Thu, Sep 7, 2023 at 12:52 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   99d99825fc075fd24b60cc9cf0fb1e20b9c16b0f
> > commit: 914d9d831e6126a6e7a92e27fcfaa250671be42c of: dynamic: Refactor action prints to not use "%pOF" inside devtree_lock
> > config: x86_64-randconfig-161-20230831 (https://download.01.org/0day-ci/archive/20230901/202309011059.EOdr4im9-lkp@intel.com/config)
> > compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
> > reproduce: (https://download.01.org/0day-ci/archive/20230901/202309011059.EOdr4im9-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > | Closes: https://lore.kernel.org/r/202309011059.EOdr4im9-lkp@intel.com/
> >
> > smatch warnings:
> > drivers/of/dynamic.c:935 of_changeset_action() warn: possible memory leak of 'ce'
> >
> > vim +/ce +935 drivers/of/dynamic.c
> >
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  925  int of_changeset_action(struct of_changeset *ocs, unsigned long action,
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  926                struct device_node *np, struct property *prop)
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  927  {
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  928        struct of_changeset_entry *ce;
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  929
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  930        ce = kzalloc(sizeof(*ce), GFP_KERNEL);
> > 606ad42aa3b1fe Rob Herring       2016-06-15  931        if (!ce)
> > 201c910bd6898d Pantelis Antoniou 2014-07-04  932                return -ENOMEM;
> > 606ad42aa3b1fe Rob Herring       2016-06-15  933
> > 914d9d831e6126 Rob Herring       2023-08-18  934        if (WARN_ON(action >= ARRAY_SIZE(action_names)))
> > 914d9d831e6126 Rob Herring       2023-08-18 @935                return -EINVAL;
> >
> > No kfree(ce).  Probably we move this check before the kmalloc()?
> 
> Yes, moving up sounds great.
> Care to send a patch?
> 

Sure.  I can do that.

These are zero day bot warnings from Intel.  I normally just look them
over and hit the forward button.  Quite often the zero day bot will find
the warnings before the patch has been apply.  But in this case, it's
simple enough to patch.

regards,
dan carpenter