* [GIT PULL] meminit fix for v5.3-rc2 @ 2019-07-28 19:21 Kees Cook 2019-07-28 19:43 ` Linus Torvalds 2019-07-28 19:50 ` pr-tracker-bot 0 siblings, 2 replies; 8+ messages in thread From: Kees Cook @ 2019-07-28 19:21 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-kernel, Arnd Bergmann Hi Linus, Please pull this meminit fix for v5.3-rc2. This is late in the -rc2 window because I got confused about whether I or akpm was taking this patch. I noticed it still wasn't in -mm, so here it is. It's a small Kconfig change that fixes a bunch of build warnings under KASAN and the gcc-plugin-based stack auto-initialization features (which are arguably redundant, so better to let KASAN control this). Thanks! -Kees The following changes since commit cd6c84d8f0cdc911df435bb075ba22ce3c605b07: Linux 5.2-rc2 (2019-05-26 16:49:19 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/meminit-v5.3-rc2 for you to fetch changes up to 173e6ee21e2b3f477f07548a79c43b8d9cfbb37d: structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK (2019-07-25 16:16:12 -0700) ---------------------------------------------------------------- meminit fix - Disable gcc-based stack variable auto-init under KASAN (Arnd Bergmann) ---------------------------------------------------------------- Arnd Bergmann (1): structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK security/Kconfig.hardening | 7 +++++++ 1 file changed, 7 insertions(+) -- Kees Cook ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-28 19:21 [GIT PULL] meminit fix for v5.3-rc2 Kees Cook @ 2019-07-28 19:43 ` Linus Torvalds 2019-07-28 19:50 ` Linus Torvalds 2019-07-28 22:16 ` Kees Cook 2019-07-28 19:50 ` pr-tracker-bot 1 sibling, 2 replies; 8+ messages in thread From: Linus Torvalds @ 2019-07-28 19:43 UTC (permalink / raw) To: Kees Cook; +Cc: Linux List Kernel Mailing, Arnd Bergmann On Sun, Jul 28, 2019 at 12:21 PM Kees Cook <keescook@chromium.org> wrote: > > Please pull this meminit fix for v5.3-rc2. Side noe: I find "meminit" a confusing description for the structleak thing. When I hear it, it sounds like some generic memory initialization thing in the VM layer (which we obviously do also have), not the stack variable initialization. Also, have you guys talked to gcc people about just making it a real feature, like I think it is for clang? In particular, I still suspect that we could/should just make zero-filling the *default* in the long run, and say "our C standard is that local variables are initialized to zero, exactly the same way static variables are". I know you posted some numbers somewhere (well, I'm pretty sure you did) and the full stack initialization really was pretty cheap, wasn't it? Linus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-28 19:43 ` Linus Torvalds @ 2019-07-28 19:50 ` Linus Torvalds 2019-07-28 22:16 ` Kees Cook 1 sibling, 0 replies; 8+ messages in thread From: Linus Torvalds @ 2019-07-28 19:50 UTC (permalink / raw) To: Kees Cook; +Cc: Linux List Kernel Mailing, Arnd Bergmann On Sun, Jul 28, 2019 at 12:43 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Side note: I find "meminit" a confusing description for the structleak > thing. Not that 'structleak' is all that much better, because it's not necessarily just about structs either. 'stack variable initialization' is too long. I dunno. But it's more about "variables" (and in the case of kmalloc etc - perhaps 'allocations') than "memory", I feel. The point is that in the kernel we do memory management and memory initialization, and that's something very different. Linus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-28 19:43 ` Linus Torvalds 2019-07-28 19:50 ` Linus Torvalds @ 2019-07-28 22:16 ` Kees Cook 2019-07-30 13:53 ` Alexander Potapenko 1 sibling, 1 reply; 8+ messages in thread From: Kees Cook @ 2019-07-28 22:16 UTC (permalink / raw) To: Linus Torvalds Cc: Linux List Kernel Mailing, Arnd Bergmann, Alexander Potapenko On Sun, Jul 28, 2019 at 12:43:15PM -0700, Linus Torvalds wrote: > On Sun, Jul 28, 2019 at 12:21 PM Kees Cook <keescook@chromium.org> wrote: > > > > Please pull this meminit fix for v5.3-rc2. > > Side noe: I find "meminit" a confusing description for the structleak > thing. When I hear it, it sounds like some generic memory > initialization thing in the VM layer (which we obviously do also > have), not the stack variable initialization. I will find a better name. :) We dreamed up "meminit" as finding a name for the umbrella of both stack and heap auto-initialization. But I agree, it's confusing. > Also, have you guys talked to gcc people about just making it a real > feature, like I think it is for clang? In particular, I still suspect > that we could/should just make zero-filling the *default* in the long > run, and say "our C standard is that local variables are initialized > to zero, exactly the same way static variables are". Yes, this is on the list for discussion at Plumber's. Having gcc do auto-init is the first part. Convincing Clang that _zero_ init isn't a language-breaking change is the second part. :P That's been a whole other issue. > I know you posted some numbers somewhere (well, I'm pretty sure you > did) and the full stack initialization really was pretty cheap, > wasn't it? Yes, Clang's initialization (which is 0xAA not 0x00 in most cases) is cheap. There are rumors(?) of some pathological workloads, though. I haven't seen real numbers for that though. I'll try to find the Clang numbers (maybe Alexander has them?) but I remember it being the same as (or maybe better than) the gcc-plugin version, which I measured here: https://git.kernel.org/linus/81a56f6dcd20 -- Kees Cook ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-28 22:16 ` Kees Cook @ 2019-07-30 13:53 ` Alexander Potapenko 2019-07-30 19:53 ` Linus Torvalds 0 siblings, 1 reply; 8+ messages in thread From: Alexander Potapenko @ 2019-07-30 13:53 UTC (permalink / raw) To: Kees Cook; +Cc: Linus Torvalds, Linux List Kernel Mailing, Arnd Bergmann Hi Kees, Linus, On Mon, Jul 29, 2019 at 12:16 AM Kees Cook <keescook@chromium.org> wrote: > > On Sun, Jul 28, 2019 at 12:43:15PM -0700, Linus Torvalds wrote: > > On Sun, Jul 28, 2019 at 12:21 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > Please pull this meminit fix for v5.3-rc2. > > > > Side noe: I find "meminit" a confusing description for the structleak > > thing. When I hear it, it sounds like some generic memory > > initialization thing in the VM layer (which we obviously do also > > have), not the stack variable initialization. > > I will find a better name. :) We dreamed up "meminit" as finding a name > for the umbrella of both stack and heap auto-initialization. But I > agree, it's confusing. > > > Also, have you guys talked to gcc people about just making it a real > > feature, like I think it is for clang? In particular, I still suspect > > that we could/should just make zero-filling the *default* in the long > > run, and say "our C standard is that local variables are initialized > > to zero, exactly the same way static variables are". I wonder how hard it should be to make a zero-filling GCC plugin? I'm not a big fan of hacking GCC, but it shouldn't differ much from the existing GCC plugins that initialize locals. > Yes, this is on the list for discussion at Plumber's. Having gcc do > auto-init is the first part. Convincing Clang that _zero_ init isn't > a language-breaking change is the second part. :P That's been a whole > other issue. > > > I know you posted some numbers somewhere (well, I'm pretty sure you > > did) and the full stack initialization really was pretty cheap, > > wasn't it? > > Yes, Clang's initialization (which is 0xAA not 0x00 in most cases) is > cheap. There are rumors(?) of some pathological workloads, though. I > haven't seen real numbers for that though. > > I'll try to find the Clang numbers (maybe Alexander has them?) but I > remember it being the same as (or maybe better than) the gcc-plugin > version, which I measured here: I've some stale data collected on an x86 QEMU instance. For 0x00 stack initialization: - hackbench, netperf and parallel Linux build were virtually free (slowdown within stdev) - for af_inet_loopback the slowdown was ~4% For 0xAA stack initialization: - netperf and parallel Linux build were free - for hackbench the slowdown was ~1.5% - for af_inet_loopback the slowdown was ~7% Since then we've made some improvements to Clang (and more are coming, e.g. cross-function DSE in https://reviews.llvm.org/D61879), so I expect the performance numbers to be better now. A big chunk of slowdown had been caused by DSE not working well with inline assembly on x86, need to check what's the current status there. Lately I've been mostly benchmarking Android system performance using the hwuimacro benchmark suite (an end-to-end benchmark for various UI features). Most of the benchmarks are unaffected by stack initialization, however there are cases in which the slowdown is up to 5% for no obvious reason (the hottest functions don't have initialization code in them, slowdown could be related to increased icache pressure). The biggest problem is that there's no single slowdown number to report. Benchmarks like netperf may show big slowdowns, but many systems don't run under netperf-like load 24/7 For graphical apps it's critical that the user doesn't notice the UI glitches introduced by the instrumentation, so benchmarks exploiting the full graphical stack are a lot more interesting. Those often have big variance though, and are very specific to the particular system. Alex > https://git.kernel.org/linus/81a56f6dcd20 > > -- > Kees Cook -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-30 13:53 ` Alexander Potapenko @ 2019-07-30 19:53 ` Linus Torvalds 2019-07-31 11:30 ` Alexander Potapenko 0 siblings, 1 reply; 8+ messages in thread From: Linus Torvalds @ 2019-07-30 19:53 UTC (permalink / raw) To: Alexander Potapenko; +Cc: Kees Cook, Linux List Kernel Mailing, Arnd Bergmann On Tue, Jul 30, 2019 at 6:53 AM Alexander Potapenko <glider@google.com> wrote: > > I wonder how hard it should be to make a zero-filling GCC plugin? > I'm not a big fan of hacking GCC, but it shouldn't differ much from > the existing GCC plugins that initialize locals. The thing is, as long as it's a plugin, I don't think we can rely on it. The gcc people will rightly just laugh at us if we were to report a bug with some kernel plugin. So I'd like the zeroing of local variables to be a native compiler option, so that we can (_eventually_ - these things take a long time) just start saying "ok, we simply consider stack variables to be always initialized". > I've some stale data collected on an x86 QEMU instance. > For 0x00 stack initialization: > - hackbench, netperf and parallel Linux build were virtually free > (slowdown within stdev) > - for af_inet_loopback the slowdown was ~4% > For 0xAA stack initialization: > - netperf and parallel Linux build were free > - for hackbench the slowdown was ~1.5% > - for af_inet_loopback the slowdown was ~7% So I would expect that we have some special cases where we end up having arrays (or big structures) on the stack that end up being critical, and where initializing them is clearly abad idea. Then we can verify manually are very much initialized, and that we could then mark and say "this is uninitialized". So when a compiler has an option to initialize stack variables, it would probably _also_ be a very good idea for that compiler to then support a variable attribute that says "don't initialize _this_ variable, I will do that manually". But if we in ten years had a kernel model where only allocations and variables that were _explicitly_ uninitialized, that would be lovely. Then you can grep for those and verify that "yes, this is safe". We've historically had the reverse model - things are uninitialized by default, and you have to explicitly initialize them. Turning that on its head is what I would like to do long-term. (For normal allocations that wouldn't be too bad: get rid of __GFP_ZERO and friends, and instead do __GFP_UNINITIALIZED). Again - I don't think we want a world where everything is force-initialized. There _are_ going to be situations where that just hurts too much. But if we get to a place where we are zero-initialized by default, and have to explicitly mark the unsafe things (and we'll have comments not just about how they get initialized, but also about why that particular thing is so performance-critical), that would be a good place to be. This, btw, is why I also think that the "initialize with poison" is pointless and wrong. Yes, it can find bugs, but it doesn't really help improve the general situation, and people see it as a debugging tool, not a "improve code quality and improve the life of kernel developers" tool. Linus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-30 19:53 ` Linus Torvalds @ 2019-07-31 11:30 ` Alexander Potapenko 0 siblings, 0 replies; 8+ messages in thread From: Alexander Potapenko @ 2019-07-31 11:30 UTC (permalink / raw) To: Linus Torvalds; +Cc: Kees Cook, Linux List Kernel Mailing, Arnd Bergmann On Tue, Jul 30, 2019 at 9:54 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Tue, Jul 30, 2019 at 6:53 AM Alexander Potapenko <glider@google.com> wrote: > > > > I wonder how hard it should be to make a zero-filling GCC plugin? > > I'm not a big fan of hacking GCC, but it shouldn't differ much from > > the existing GCC plugins that initialize locals. > > The thing is, as long as it's a plugin, I don't think we can rely on > it. The gcc people will rightly just laugh at us if we were to report > a bug with some kernel plugin. > > So I'd like the zeroing of local variables to be a native compiler > option, so that we can (_eventually_ - these things take a long time) > just start saying "ok, we simply consider stack variables to be always > initialized". > > > I've some stale data collected on an x86 QEMU instance. > > For 0x00 stack initialization: > > - hackbench, netperf and parallel Linux build were virtually free > > (slowdown within stdev) > > - for af_inet_loopback the slowdown was ~4% > > For 0xAA stack initialization: > > - netperf and parallel Linux build were free > > - for hackbench the slowdown was ~1.5% > > - for af_inet_loopback the slowdown was ~7% > > So I would expect that we have some special cases where we end up > having arrays (or big structures) on the stack that end up being > critical, and where initializing them is clearly abad idea. > > Then we can verify manually are very much initialized, and that we > could then mark and say "this is uninitialized". > > So when a compiler has an option to initialize stack variables, it > would probably _also_ be a very good idea for that compiler to then > support a variable attribute that says "don't initialize _this_ > variable, I will do that manually". FWIW Clang has __attribute((uninitialized)) already: https://reviews.llvm.org/rL349442 I agree that making it in GCC is easier if initialization itself is implemented as part of GCC. > But if we in ten years had a kernel model where only allocations and > variables that were _explicitly_ uninitialized, that would be lovely. > > Then you can grep for those and verify that "yes, this is safe". > > We've historically had the reverse model - things are uninitialized by > default, and you have to explicitly initialize them. Turning that on > its head is what I would like to do long-term. > > (For normal allocations that wouldn't be too bad: get rid of > __GFP_ZERO and friends, and instead do __GFP_UNINITIALIZED). There've been concerns about such flags easily going out of control (my original proposal for heap initialization contained such a flag). To some extent their spread can be prevented by build-time checks, but a simple grep can be insufficient, as people will start creating helper functions to allocate with __GFP_UNINITIALIZED. > Again - I don't think we want a world where everything is > force-initialized. There _are_ going to be situations where that just > hurts too much. But if we get to a place where we are zero-initialized > by default, and have to explicitly mark the unsafe things (and we'll > have comments not just about how they get initialized, but also about > why that particular thing is so performance-critical), that would be a > good place to be. > > This, btw, is why I also think that the "initialize with poison" is > pointless and wrong. Yes, it can find bugs, but it doesn't really help > improve the general situation, and people see it as a debugging tool, > not a "improve code quality and improve the life of kernel developers" > tool. This sure makes sense. If this policy is adopted kernel-wide, we won't need any debugging tools for uninit variables, so it's natural to initialize them to zero. > Linus -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [GIT PULL] meminit fix for v5.3-rc2 2019-07-28 19:21 [GIT PULL] meminit fix for v5.3-rc2 Kees Cook 2019-07-28 19:43 ` Linus Torvalds @ 2019-07-28 19:50 ` pr-tracker-bot 1 sibling, 0 replies; 8+ messages in thread From: pr-tracker-bot @ 2019-07-28 19:50 UTC (permalink / raw) To: Kees Cook; +Cc: Linus Torvalds, linux-kernel, Arnd Bergmann The pull request you sent on Sun, 28 Jul 2019 12:21:34 -0700: > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/meminit-v5.3-rc2 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/c622fc5f54cb0c7ea2e6fedba27ba533b97657d8 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-31 11:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-28 19:21 [GIT PULL] meminit fix for v5.3-rc2 Kees Cook 2019-07-28 19:43 ` Linus Torvalds 2019-07-28 19:50 ` Linus Torvalds 2019-07-28 22:16 ` Kees Cook 2019-07-30 13:53 ` Alexander Potapenko 2019-07-30 19:53 ` Linus Torvalds 2019-07-31 11:30 ` Alexander Potapenko 2019-07-28 19:50 ` pr-tracker-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.