Message ID | 20221008132113.919b9b894426297de78ac00f@linux-foundation.org (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [GIT,PULL] MM updates for 6.1-rc1 | expand |
On Sat, Oct 8, 2022 at 1:21 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > Alexander Potapenko (43): > crypto: kmsan: disable accelerated configs under KMSAN Ok, so this conflicted pretty badly with the crypto tree having re-organized their Kconfig files. I channelled my inner Alexander the Great, and just cut the whole thing through by disabling all the architecture-accelerated crypto includes when KMSAN was enabled. But that's very different from your tree that disabled the options individually, so some KMSAN person should probably look around at it and decide whether some more subtle approach is called for. The other thing I notice from just doing a build is that I now get vmlinux.o: warning: objtool: kasan_report+0x12: call to stackleak_track_stack() with UACCESS enabled which may just be a "need to teach objtool this is ok", but I'm not seeing why it's now starting to happen. Maybe just code generation changed for some reason, or maybe it's just that config options ended up changing enough to expose this on my allmodconfig builds. Cc'ing people who may know better. This is just my reaction from doing the merge and test-build. There may or may not be more commentary once I've actually tried to boot into this thing. Linus
The pull request you sent on Sat, 8 Oct 2022 13:21:13 -0700:
> https://lkml.kernel.org/r/20221004204025.7be8a3be@canb.auug.org.au Thanks.
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/27bc50fc90647bbf7b734c3fc306a5e61350da53
Thank you!
On Mon, Oct 10, 2022 at 06:20:00PM -0700, Linus Torvalds wrote: > The other thing I notice from just doing a build is that I now get > > vmlinux.o: warning: objtool: kasan_report+0x12: call to > stackleak_track_stack() with UACCESS enabled So kasan_report() is already marked as being special; and it does the mandatory user_access_save() / user_access_restore() things to fix it up. But it looks like kasan code itself is now getting instrumented by the stackleak stuff and that inserts a call outside of the user_access_save()/restore() thing, and *that* is getting flagged. Looking at mm/kasan/Makefile it disables a lot of the instrumentation, but perhaps not enough? I've not yet tried to reproduce, I'm taking this is allyesconfig or something glorious like that? LLVM-15 ?
On Tue, Oct 11, 2022 at 3:20 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sat, Oct 8, 2022 at 1:21 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > Alexander Potapenko (43): > > crypto: kmsan: disable accelerated configs under KMSAN > > Ok, so this conflicted pretty badly with the crypto tree having > re-organized their Kconfig files. Sorry about that. There was a fix for this problem: https://lore.kernel.org/lkml/20220909095811.2166073-2-glider@google.com/, but I am not sure where it ended up, given that crypto tree didn't have KMSAN, and -mm didn't have the crypto changes. > I channelled my inner Alexander the Great, and just cut the whole > thing through by disabling all the architecture-accelerated crypto > includes when KMSAN was enabled. That's exactly the point. Thanks for doing that! > But that's very different from your tree that disabled the options > individually, so some KMSAN person should probably look around at it > and decide whether some more subtle approach is called for. No, in fact it was pretty painful to disable the configs one by one. Thankfully, the new Kconfig layout simplifies the things for us. > The other thing I notice from just doing a build is that I now get > > vmlinux.o: warning: objtool: kasan_report+0x12: call to > stackleak_track_stack() with UACCESS enabled Guess that's unrelated to KMSAN. Happy to take a look anyway. > which may just be a "need to teach objtool this is ok", but I'm not > seeing why it's now starting to happen. Maybe just code generation > changed for some reason, or maybe it's just that config options ended > up changing enough to expose this on my allmodconfig builds. > > Cc'ing people who may know better. > > This is just my reaction from doing the merge and test-build. There > may or may not be more commentary once I've actually tried to boot > into this thing. > > Linus Small nit: - Dmitry Vyukov introduces KMSAN: the Kernel Memory Sanitizer. It uses clang-generated instrumentation to detect used-unintialized bugs down to the single bit level. s/Dmitry Vyukov/Alexander Potapenko/ ;) (although contributions of Dmitry Vyukov and Marco Elver to the whole thing cannot be underestimated!). -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
On Tue, Oct 11, 2022 at 1:24 AM Peter Zijlstra <peterz@infradead.org> wrote: > > I've not yet tried to reproduce, I'm taking this is allyesconfig or > something glorious like that? LLVM-15 ? Just a bog-standard allmodconfig build with gcc 12.2.1 as packaged by Fedora. Linus
On Tue, Oct 11, 2022 at 2:03 AM Alexander Potapenko <glider@google.com> wrote: > > Small nit: > > - Dmitry Vyukov introduces KMSAN: the Kernel Memory Sanitizer. It uses > clang-generated instrumentation to detect used-unintialized bugs down > to the single bit level. > > s/Dmitry Vyukov/Alexander Potapenko/ ;) Uhhuh. I just took what Andrew told me for the commit message, so we have that error in the history now. Then when I reported my conflict resolution, at that point I actually looked at the commit that introduced it, which is why I cc'd you. So you got all the blame, and none of the glory. Sorry about that, Linus
On Tue, Oct 11, 2022 at 10:24:27AM +0200, Peter Zijlstra wrote: > On Mon, Oct 10, 2022 at 06:20:00PM -0700, Linus Torvalds wrote: > > The other thing I notice from just doing a build is that I now get > > > > vmlinux.o: warning: objtool: kasan_report+0x12: call to > > stackleak_track_stack() with UACCESS enabled > > So kasan_report() is already marked as being special; and it does the > mandatory user_access_save() / user_access_restore() things to fix it > up. > > But it looks like kasan code itself is now getting instrumented by the > stackleak stuff and that inserts a call outside of the > user_access_save()/restore() thing, and *that* is getting flagged. > > Looking at mm/kasan/Makefile it disables a lot of the instrumentation, > but perhaps not enough? I can recreate, but weirdly the below doesn't seem to fix it. diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile index d4837bff3b60..a41cf1235032 100644 --- a/mm/kasan/Makefile +++ b/mm/kasan/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 KASAN_SANITIZE := n +KMSAN_SANITIZE := n UBSAN_SANITIZE := n KCOV_INSTRUMENT := n
On Tue, Oct 11, 2022 at 11:44:46AM -0700, Josh Poimboeuf wrote: > On Tue, Oct 11, 2022 at 10:24:27AM +0200, Peter Zijlstra wrote: > > On Mon, Oct 10, 2022 at 06:20:00PM -0700, Linus Torvalds wrote: > > > The other thing I notice from just doing a build is that I now get > > > > > > vmlinux.o: warning: objtool: kasan_report+0x12: call to > > > stackleak_track_stack() with UACCESS enabled > > > > So kasan_report() is already marked as being special; and it does the > > mandatory user_access_save() / user_access_restore() things to fix it > > up. > > > > But it looks like kasan code itself is now getting instrumented by the > > stackleak stuff and that inserts a call outside of the > > user_access_save()/restore() thing, and *that* is getting flagged. > > > > Looking at mm/kasan/Makefile it disables a lot of the instrumentation, > > but perhaps not enough? > > I can recreate, but weirdly the below doesn't seem to fix it. Duh, because kmsan != stackleak, obviously... > > diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile > index d4837bff3b60..a41cf1235032 100644 > --- a/mm/kasan/Makefile > +++ b/mm/kasan/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > KASAN_SANITIZE := n > +KMSAN_SANITIZE := n > UBSAN_SANITIZE := n > KCOV_INSTRUMENT := n > >