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: <d2688eb2-d7b6-4e80-a13e-55ed541ac9b8@kili.mountain>
Date:   Sat, 1 Apr 2023 11:22:52 +0300
From:   Dan Carpenter <error27@...il.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Li Yang <lidaxian@...t.edu.cn>,
        Magnus Damm <magnus.damm@...il.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Biju Das <biju.das.jz@...renesas.com>,
        linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc: renesas: renesas-soc: release 'chipid' from
 ioremap()

On Fri, Mar 31, 2023 at 02:13:10PM +0200, Geert Uytterhoeven wrote:
> Hi Li,
> 
> On Fri, Mar 31, 2023 at 12:14 PM Li Yang <lidaxian@...t.edu.cn> wrote:
> > Smatch reports:
> >
> > drivers/soc/renesas/renesas-soc.c:536 renesas_soc_init() warn:
> > 'chipid' from ioremap() not released on lines: 475.
> >
> > If soc_dev_atrr allocation is failed, function renesas_soc_init()
> > will return without releasing 'chipid' from ioremap().
> >
> > Fix this by adding function iounmap().
> >
> > Fixes: cb5508e47e60 ("soc: renesas: Add support for reading product revision for RZ/G2L family")
> > Signed-off-by: Li Yang <lidaxian@...t.edu.cn>
> > Reviewed-by: Dan Carpenter <error27@...il.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/soc/renesas/renesas-soc.c
> > +++ b/drivers/soc/renesas/renesas-soc.c
> > @@ -471,8 +471,11 @@ static int __init renesas_soc_init(void)
> >         }
> >
> >         soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > -       if (!soc_dev_attr)
> > +       if (!soc_dev_attr) {
> > +               if (chipid)
> > +                       iounmap(chipid);
> 
> We don't really care, as the system is dead at this point anyway.
> 

Why even have the check for NULL then?  The kzalloc() is small enough
to the point where it litterally cannot fail.

This patch is already written, it's way less effort for us to apply it
than it is to debate its worth.  I kind of feel like it's better to just
fix the bugs even when they don't affect real life.  Otherwise it's a
constant debate with bugs if they're worth fixing.

This is a university group and they're looking for bugs to fix.  I'm
like, "I'm drowning in resource leak warnings and I don't even look at
them any more because they're basically all trash that no one cares
about."  So I was hoping to maybe clean up the trash a bit to the point
where we can start caring about the leaks.

regards,
dan carpenter

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ