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: <CAHk-=wg1LeRWPmA-2pit+aH0LKZoBbhnGzmwWrPz6bbchMkCzw@mail.gmail.com> Date: Sun, 17 Jul 2022 18:38:56 -0700 From: Linus Torvalds <torvalds@...ux-foundation.org> To: Segher Boessenkool <segher@...nel.crashing.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 2:49 PM Segher Boessenkool <segher@...nel.crashing.org> wrote: > > > 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. That is not actually a valid or realistic argument in the general case. The thing is, an operation on an aggregate type in C is fundamentally different from the "do the same operation on the individual parts of the struct". Just to make a very concrete example of that, it's not at all unreasonable to have a struct like this: struct io_accessor { union { volatile unsigned char byte[8]; volatile unsigned short word[4]; ... and while that wasn't the example here, it's not completely insane as a concept to use as a helper type so that you can do volatile accesses of different sizes. And while you'd be right to say that "assigning that kind of struct is probably insane", and I wouldn't argue against it, I also think that basically *any* union member is basically an argument that a structure assignment is *NOT* about "assign all the individual members", and never really can be. In the above union, make one union member be a non-volatile type, and suddenly it actually *can* be ok to copy the struct. Even though it also has volatile bytes. So once you start doing structure assignments but argue about individual fields being volatile, I think you're on very shaky ground. And I think "memcpy" is a reasonable way to say "we don't care - and in the general case we CANNOT know - what the individual members are, so we'll just copy it as one thing". So the compiler emitting a "memcpy()" to assign a structure sounds fine. Even in theory. Because the "but individual fields.." argument just cannot work in general. In contrast, when you access the members individually (like the kernel does in this powerpc case), there is no such ambiguity. There is no way in hell that it is ever ok to do a "memcpy()" when the user has done the assignments one volatile member at a time. So that's why I don't think your test-case with the struct assignment is very good. I think it's very reasonable for a compiler person to say "you assigned the whole struct, you get what you asked for, you get a memcpy". But when you do a loop that assigns individual volatile fields? No such problem. Completely unambiguous that you need to do them one at a time as individual accesses. Linus
Powered by blists - more mailing lists