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
| ||
|
Message-ID: <20220717214508.GD25951@gate.crashing.org> Date: Sun, 17 Jul 2022 16:45:08 -0500 From: Segher Boessenkool <segher@...nel.crashing.org> To: Linus Torvalds <torvalds@...ux-foundation.org> Cc: Sudip Mukherjee <sudipm.mukherjee@...il.com>, Michael Ellerman <mpe@...erman.id.au>, Benjamin Herrenschmidt <benh@...nel.crashing.org>, Paul Mackerras <paulus@...ba.org>, Kees Cook <keescook@...omium.org>, linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>, linux-kernel <linux-kernel@...r.kernel.org>, linux-hardening@...r.kernel.org Subject: Re: mainline build failure of powerpc allmodconfig for prom_init_check On Sun, Jul 17, 2022 at 02:11:52PM -0700, Linus Torvalds wrote: > On Sun, Jul 17, 2022 at 2:00 PM Segher Boessenkool > <segher@...nel.crashing.org> wrote: > > Calling mem* on a volatile object (or a struct containing one) is not > > valid. I opened gcc.gnu.org/PR106335. > > Well, that very quickly got marked as a duplicate of a decade-old bug. > > So I guess we shouldn't expect this to be fixed any time soon. It shouldn't be all that hard to implement. GCC wants all ports to define their own mem* because these functions are so critical for performance, but it isn't hard to do a straightforward by-field copy for assignments if using memcpy would not be valid at all. Also, if we would have this we could make a compiler flag saying to always open-code this, getting rid of this annoyance (namely, that extetnal mem* are required) for -ffreestanding. > That said, your test-case of copying the whole structure is very > different from the one in the kernel that works on them one structure > member at a time. > > I can *kind of* see the logic that when you do a whole struct > assignment, it turns into a "memcpy" without regard for volatile > members. You're not actually accessing the volatile members in some > particular order, so the struct assignment arguably does not really > have an access ordering that needs to be preserved. The order is not defined, correct. But a "volatile int" can only be accessed as an int, and an external memcpy will typically use different size accesses, and can even access some fields more than once (or partially); all not okay for a volatile object. > But the kernel code in question very much does access the members > individually, and so I think that the compiler quite unequivocally did > something horribly horribly bad by turning them into a memset. > > So I don't think your test-case is really particularly good, and maybe > that's why that old bug has languished for over a decade - people > didn't realize just *how* incredibly broken it was. People haven't looked at my test case for all that time, it sprouted from my demented mind just minutes ago ;-) The purpose of writing it this way was to make sure that memcpy will be called for this (on any target etc.), not some shorter and/or smarter thing. I don't know what the real reason is that this bugs hasn't been fixed yet. It should be quite easy to make this more correct. In <https://patchwork.ozlabs.org/project/gcc/patch/1408617247-21558-1-git-send-email-james.greenhalgh@arm.com/#843066> Richard suggested doing it in the frontend, which seems reasonable (but more work than the patch there). There have been no follow-up patches as far as I can see :-( Segher
Powered by blists - more mailing lists