linux-kernel - Re: [PATCH v2] fs-fat: Less function calls in fat_fill

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 1 Dec 2014 09:43:50 +0100 (CET)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Dan Carpenter <dan.carpenter@...cle.com>
cc:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	Julia Lawall <julia.lawall@...6.fr>,
	SF Markus Elfring <elfring@...rs.sourceforge.net>,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
	trivial@...nel.org, Coccinelle <cocci@...teme.lip6.fr>
Subject: Re: [PATCH v2] fs-fat: Less function calls in fat_fill_super() after
 error detection



On Mon, 1 Dec 2014, Dan Carpenter wrote:

> On Sat, Nov 29, 2014 at 10:59:47PM +0900, OGAWA Hirofumi wrote:
> > Julia Lawall <julia.lawall@...6.fr> writes:
> > 
> > >> iput() checks NULL of inode. What is wrong just remove NULL check,
> > >> instead of adding new jump labels?
> > >
> > > Personally, I prefer that code that can be statically determined not to
> > > need to be executed not to be executed.  It can make the code easier to
> > > understand, because each function is only called when doing so is useful,
> > > and it can be helpful to static analysis.
> > 
> > Hm, first of all, we want to prevent the bugs. More labels are more
> > chances of bug (and we don't care micro optimize on this error path),
> > isn't it?  Increasing the chance of bugs and bothers developers for
> > analyzer sounds like strange.
> 
> Oh wow!  Absolutely not.  "One Err Bugs" are one of the most common
> kinds of bugs we have in the kernel.  This is where you have just one
> error label and the bugs look like this:
> 
> err:
> 	kfree(foo->bar);
> 	kfree(foo);
> 
> but foo is NULL.  Mixing the error paths together it always creates
> confusion.  I fix so many of these bugs...  We get a few new ones every
> week.

Just for concreteness, from drivers/clk/st/clkgen-mux.c (- indicates 
lines of interest, not lines to remove):

@@ -722,7 +722,6 @@ void __init st_of_clkgen_vcc_setup(struc
                return;

        clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
-       if (!clk_data)
                goto err;

        clk_data->clk_num = VCC_MAX_CHANNELS;
@@ -808,7 +807,6 @@ void __init st_of_clkgen_vcc_setup(struc
        return;

 err:
-       for (i = 0; i < clk_data->clk_num; i++) {
                struct clk_composite *composite;

                if (!clk_data->clks[i])

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/