linux-kernel - Re: [PATCH -next] staging: greybus: fix a missing-check bug in gb_lights_light

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]

Date:   Thu, 2 Apr 2020 14:16:18 +0100
From:   Rui Miguel Silva <rmfrfs@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Chen Zhou <chenzhou10@...wei.com>, johan@...nel.org,
        elder@...nel.org, gregkh@...uxfoundation.org,
        greybus-dev@...ts.linaro.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next] staging: greybus: fix a missing-check bug in
 gb_lights_light_config()

Hi Dan,

On Thu, Apr 02, 2020 at 03:22:28PM +0300, Dan Carpenter wrote:
> On Wed, Apr 01, 2020 at 11:00:17AM +0800, Chen Zhou wrote:
> > In gb_lights_light_config(), 'light->name' is allocated by kstrndup().
> > It returns NULL when fails, add check for it.
> > 
> > Signed-off-by: Chen Zhou <chenzhou10@...wei.com>
> > ---
> >  drivers/staging/greybus/light.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> > index d6ba25f..d2672b6 100644
> > --- a/drivers/staging/greybus/light.c
> > +++ b/drivers/staging/greybus/light.c
> > @@ -1026,7 +1026,8 @@ static int gb_lights_light_config(struct gb_lights *glights, u8 id)
> >  
> >  	light->channels_count = conf.channel_count;
> >  	light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
> > -
> > +	if (!light->name)
> > +		return -ENOMEM;
> >  	light->channels = kcalloc(light->channels_count,
> >  				  sizeof(struct gb_channel), GFP_KERNEL);
> >  	if (!light->channels)
> 
> The clean up in this function is non-existant.  :(

Yeah, this have a central point to do the cleanups, gb_lights_release,
since we may have other lights already configured at this point, we
could cleanup this specific one here, but than would need to make sure
all other already configure got clean also.

------
Cheers,
     Rui