linux-kernel - Re: [PATCH] mtd: spi-nor: avoid holes in struct spi_mem

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: <598bd9d8-249e-125c-bde3-7a63ba6dc5f7@linaro.org>
Date:   Thu, 20 Jul 2023 07:50:33 +0100
From:   Tudor Ambarus <tudor.ambarus@...aro.org>
To:     Arnd Bergmann <arnd@...nel.org>,
        Pratyush Yadav <pratyush@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>
Cc:     Arnd Bergmann <arnd@...db.de>, Peter Foley <pefoley2@...oley.com>,
        Pedro Falcato <pedro.falcato@...il.com>,
        Michael Walle <michael@...le.cc>,
        Mark Brown <broonie@...nel.org>,
        Takahiro Kuwano <Takahiro.Kuwano@...ineon.com>,
        Dhruva Gole <d-gole@...com>, linux-mtd@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org
Subject: Re: [PATCH] mtd: spi-nor: avoid holes in struct spi_mem_op



On 7/19/23 20:00, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> gcc gets confused when -ftrivial-auto-var-init=pattern is used on sparse
> bit fields such as 'struct spi_mem_op', which caused the previous false
> positive warning about an uninitialized variable:
> 
> drivers/mtd/spi-nor/spansion.c: error: 'op' is used uninitialized [-Werror=uninitialized]
> 
> In fact, the variable is fully initialized and gcc does not see it being
> used, so the warning is entirely bogus. The problem appears to be
> a misoptimization in the initialization of single bit fields when the
> rest of the bytes are not initialized.
> 
> A previous workaround added another initialization, which ended up
> shutting up the warning in spansion.c, though it apparently still happens
> in other files as reported by Peter Foley in the gcc bugzilla. The
> workaround of adding a fake initialization seems particularly bad
> because it would set values that can never be correct but prevent the
> compiler from warning about actually missing initializations.
> 
> Revert the broken workaround and instead pad the structure to only
> have bitfields that add up to full bytes, which should avoid this
> behavior in all drivers.
> 
> I also filed a new bug against gcc with what I found, so this can
> hopefully be addressed in future gcc releases. At the moment, only
> gcc-12 and gcc-13 are affected.
> 
> Cc: Peter Foley <pefoley2@...oley.com>
> Cc: Pedro Falcato <pedro.falcato@...il.com>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110743
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108402
> Link: https://godbolt.org/z/efMMsG1Kx
> Fixes: 420c4495b5e56 ("mtd: spi-nor: spansion: make sure local struct does not contain garbage")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Acked-by: Tudor Ambarus <tudor.ambarus@...aro.org>

Miquel, would you please take this through mtd/fixes?

Cheers,
ta