linux-kernel - Re: [PATCH] sdhci: 'scratch' may be used uninitialized

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:	Mon, 6 Oct 2008 08:59:21 +0300
From:	Adrian Bunk <bunk@...nel.org>
To:	Steven Noonan <steven@...inklabs.net>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, drzeus@...eus.cx
Subject: Re: [PATCH] sdhci: 'scratch' may be used uninitialized

On Sun, Oct 05, 2008 at 04:48:49PM -0700, Steven Noonan wrote:
> On Sun, Oct 5, 2008 at 4:16 PM, Adrian Bunk <bunk@...nel.org> wrote:
> > On Sun, Oct 05, 2008 at 03:53:28PM -0700, Steven Noonan wrote:
> >> On Sun, Oct 5, 2008 at 7:28 AM, Adrian Bunk <bunk@...nel.org> wrote:
> >> > On Wed, Oct 01, 2008 at 01:50:25AM -0700, Steven Noonan wrote:
> >> >> The variable 'scratch' is always initialized before it's used. The
> >> >> conditional which is responsible for initialization of 'scratch' will
> >> >> always evaluate 'true' when the first loop iteration occurs, and thus,
> >> >> it's properly initialized. GCC doesn't see this, of course, so using
> >> >> the uninitialized_var() macro seems to work for silencing this case.
> >> >>
> >> >> Signed-off-by: Steven Noonan <steven@...inklabs.net>
> >> >> ---
> >> >>  drivers/mmc/host/sdhci.c |    2 +-
> >> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >> >>
> >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> >> index e3a8133..6257677 100644
> >> >> --- a/drivers/mmc/host/sdhci.c
> >> >> +++ b/drivers/mmc/host/sdhci.c
> >> >> @@ -177,7 +177,7 @@ static void sdhci_read_block_pio(struct sdhci_host *host)
> >> >>  {
> >> >>       unsigned long flags;
> >> >>       size_t blksize, len, chunk;
> >> >> -     u32 scratch;
> >> >> +     u32 uninitialized_var(scratch);
> >> >>...
> >> >
> >> > With which gcc version?
> >> >
> >> > I'm not getting this warning with gcc 4.3, and IMHO it doesn't make
> >> > sense to clutter the source code with such workarounds for older gcc
> >> > versions (we officially support 6 years old compilers, and warning-free
> >> > compilations with all of them are not reasonably possible).
> >> >
> >> > cu
> >> > Adrian
> >>
> >> I've seen it on GCC 4.1 and 4.2. Since lots of distributions still
> >> haven't marked GCC >4.1 stable, it makes sense to me to kill warnings
> >> for GCC 4.1 and above. I don't know of any current distribution
> >> releases using less than GCC 4.1 at the moment.
> >
> > It will clutter our code with these workarounds forever.
> >
> > And due to silencing these false warnings we will no longer get a
> > warning when one of them becomes a real bug.
> >
> > Working on the remaining warnings that are visible with gcc 4.3 is a
> > worthwhile goal, but I see no point for silencing some warnings that
> > only occur with older gcc versions (especially as long as warnings
> > that are present with all gcc versions stay unfixed).
> >
> I feel like there's a logical fallacy here. Sure, we can fix GCC 4.3
> warnings, but what about when GCC 4.3 becomes an "old version"?
> uninitialized_var and other such workarounds will still exist in the
> code. It seems like the logical progression of your argument should be
> to never fix false warnings.

There is no logical fallacy here - getting a warning-free compilation 
with the latest gcc (so that new warnings get more obvious) makes sense,
but workarounds for warnings only present with older gcc versions are 
not worth the price.

If compilation with gcc 4.3 was always warning-free you might have a 
point, but considering that it's unlikely that we get all warnings with 
gcc 4.3 fixed in the forseeable future [1] it would make more sense to 
fix one of the non-trivial warnings present with all gcc version than 
clutter the code with workarounds for warnings only present with older 
gcc versions.

> - Steven

cu
Adrian

[1] e.g. the MCA legacy stuff gives #warning's since
    more than 5 years (sic)

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--
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/