| Message ID | 20240820194910.187826-1-mmaurer@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Rust KASAN Support | expand |
On Tue, Aug 20, 2024 at 9:49 PM Matthew Maurer <mmaurer@google.com> wrote: > > Right now, if we turn on KASAN, Rust code will cause violations because > it's not enabled properly. > > This series: > 1. Adds flag probe macros for Rust - now that we're setting a minimum rustc > version instead of an exact one, these could be useful in general. We need > them in this patch because we don't set a restriction on which LLVM rustc > is using, which is what KASAN actually cares about. > 2. Makes `rustc` enable the relevant KASAN sanitizer flags when C does. > 3. Adds a smoke test to the `kasan_test` KUnit suite to check basic > integration. > > This patch series requires the target.json array support patch [1] as > the x86_64 target.json file currently produced does not mark itself as KASAN > capable, and is rebased on top of the KASAN Makefile rewrite [2]. > > Differences from v3 [3]: > * Probing macro comments made more accurate > * Probing macros now set --out-dir to avoid potential read-only fs > issues > * Reordered KHWASAN explicit disablement patch to come before KASAN > enablement > * Comment/ordering cleanup in KASAN makefile > * Ensured KASAN tests work with and without CONFIG_RUST enabled > > [1] https://lore.kernel.org/lkml/20240730-target-json-arrays-v1-1-2b376fd0ecf4@google.com/ > [2] https://lore.kernel.org/all/20240813224027.84503-1-andrey.konovalov@linux.dev > [3] https://lore.kernel.org/all/20240819213534.4080408-1-mmaurer@google.com/ > > Matthew Maurer (4): > kbuild: rust: Define probing macros for rustc > rust: kasan: Rust does not support KHWASAN > kbuild: rust: Enable KASAN support > kasan: rust: Add KASAN smoke test via UAF > > init/Kconfig | 1 + > mm/kasan/Makefile | 7 ++- > mm/kasan/kasan.h | 6 +++ > mm/kasan/{kasan_test.c => kasan_test_c.c} | 12 +++++ > mm/kasan/kasan_test_rust.rs | 19 ++++++++ > scripts/Kconfig.include | 8 ++++ > scripts/Makefile.compiler | 15 ++++++ > scripts/Makefile.kasan | 57 ++++++++++++++++------- > scripts/Makefile.lib | 3 ++ > scripts/generate_rust_target.rs | 1 + > 10 files changed, 112 insertions(+), 17 deletions(-) > rename mm/kasan/{kasan_test.c => kasan_test_c.c} (99%) > create mode 100644 mm/kasan/kasan_test_rust.rs > > -- > 2.46.0.184.g6999bdac58-goog > Left a couple of nit comments - feel free to ignore if you don't end up sending v5. Otherwise, for patches 2-4: Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
On Tue, Aug 20, 2024 at 9:49 PM Matthew Maurer <mmaurer@google.com> wrote: > > Right now, if we turn on KASAN, Rust code will cause violations because > it's not enabled properly. > > This series: > 1. Adds flag probe macros for Rust - now that we're setting a minimum rustc > version instead of an exact one, these could be useful in general. We need > them in this patch because we don't set a restriction on which LLVM rustc > is using, which is what KASAN actually cares about. > 2. Makes `rustc` enable the relevant KASAN sanitizer flags when C does. > 3. Adds a smoke test to the `kasan_test` KUnit suite to check basic > integration. > > This patch series requires the target.json array support patch [1] as > the x86_64 target.json file currently produced does not mark itself as KASAN > capable, and is rebased on top of the KASAN Makefile rewrite [2]. Applied to `rust-next` -- thanks everyone! [ Applied empty line nit, removed double empty line, applied `rustfmt` and formatted crate comment. - Miguel ] [ Applied "SW_TAGS KASAN" nit. - Miguel ] I think `TMPOUT` needs to be passed though, i.e. like I did in https://github.com/Rust-for-Linux/linux/pull/1087#issuecomment-2218445303: diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 057305eae85c..0ac8679095f4 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -20,6 +20,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$ # Exit code chooses option. "$$TMP" serves as a temporary file and is # automatically cleaned up. try-run = $(shell set -e; \ + TMPOUT=$(TMPOUT); \ TMP=$(TMPOUT)/tmp; \ trap "rm -rf $(TMPOUT)" EXIT; \ mkdir -p $(TMPOUT); \ Or is there something I am missing? Cheers, Miguel
On Mon, Sep 16, 2024 at 6:15 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > Applied to `rust-next` -- thanks everyone! Also, for KASAN + RETHUNK builds, I noticed objtool detects this: samples/rust/rust_print.o: warning: objtool: asan.module_ctor+0x17: 'naked' return found in MITIGATION_RETHUNK build samples/rust/rust_print.o: warning: objtool: asan.module_dtor+0x17: 'naked' return found in MITIGATION_RETHUNK build And indeed from a quick look the `ret` is there. Since KASAN support is important, I decided to take it nevertheless, but please let's make sure this is fixed during the cycle (or add a "depends on"). Thanks! Cheers, Miguel
On Mon, Sep 16, 2024 at 6:47 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Mon, Sep 16, 2024 at 6:15 PM Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: > > > > Applied to `rust-next` -- thanks everyone! > > Also, for KASAN + RETHUNK builds, I noticed objtool detects this: > > samples/rust/rust_print.o: warning: objtool: > asan.module_ctor+0x17: 'naked' return found in MITIGATION_RETHUNK > build > samples/rust/rust_print.o: warning: objtool: > asan.module_dtor+0x17: 'naked' return found in MITIGATION_RETHUNK > build > > And indeed from a quick look the `ret` is there. > > Since KASAN support is important, I decided to take it nevertheless, > but please let's make sure this is fixed during the cycle (or add a > "depends on"). I figured out what the problem is. I will follow up with a fix soon. Alice
On Wed, Sep 25, 2024 at 10:26 AM Alice Ryhl <aliceryhl@google.com> wrote: > > On Mon, Sep 16, 2024 at 6:47 PM Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: > > > > On Mon, Sep 16, 2024 at 6:15 PM Miguel Ojeda > > <miguel.ojeda.sandonis@gmail.com> wrote: > > > > > > Applied to `rust-next` -- thanks everyone! > > > > Also, for KASAN + RETHUNK builds, I noticed objtool detects this: > > > > samples/rust/rust_print.o: warning: objtool: > > asan.module_ctor+0x17: 'naked' return found in MITIGATION_RETHUNK > > build > > samples/rust/rust_print.o: warning: objtool: > > asan.module_dtor+0x17: 'naked' return found in MITIGATION_RETHUNK > > build > > > > And indeed from a quick look the `ret` is there. > > > > Since KASAN support is important, I decided to take it nevertheless, > > but please let's make sure this is fixed during the cycle (or add a > > "depends on"). > > I figured out what the problem is. I will follow up with a fix soon. I posted a fix: https://github.com/rust-lang/rust/pull/130824 We'll need a check on RUSTC_VERSION in Kconfig for this. If the PR gets merged within the next 22 days, this will land in 1.83.0. Would you like me to send a fix with that version number now or wait for it to get merged before I send that fix? Alice
On Wed, Sep 25, 2024 at 12:00 PM Alice Ryhl <aliceryhl@google.com> wrote: > > I posted a fix: > https://github.com/rust-lang/rust/pull/130824 Reviewed, tagged and added to the lists -- thanks! > We'll need a check on RUSTC_VERSION in Kconfig for this. If the PR > gets merged within the next 22 days, this will land in 1.83.0. Would > you like me to send a fix with that version number now or wait for it > to get merged before I send that fix? Perhaps it could also go into 1.82.0 since it is a fix? (there are still a couple weeks for that) In any case, I think we can put 1.83 in the fix already and modify later if needed. Even then, I am not sure if the requirement is a big deal, i.e. I guess we could keep the warning and avoid adding the restriction. But since this is for KASAN-enabled, I guess it is fine adding the restriction and being safe & proper. Cheers, Miguel
Right now, if we turn on KASAN, Rust code will cause violations because it's not enabled properly. This series: 1. Adds flag probe macros for Rust - now that we're setting a minimum rustc version instead of an exact one, these could be useful in general. We need them in this patch because we don't set a restriction on which LLVM rustc is using, which is what KASAN actually cares about. 2. Makes `rustc` enable the relevant KASAN sanitizer flags when C does. 3. Adds a smoke test to the `kasan_test` KUnit suite to check basic integration. This patch series requires the target.json array support patch [1] as the x86_64 target.json file currently produced does not mark itself as KASAN capable, and is rebased on top of the KASAN Makefile rewrite [2]. Differences from v3 [3]: * Probing macro comments made more accurate * Probing macros now set --out-dir to avoid potential read-only fs issues * Reordered KHWASAN explicit disablement patch to come before KASAN enablement * Comment/ordering cleanup in KASAN makefile * Ensured KASAN tests work with and without CONFIG_RUST enabled [1] https://lore.kernel.org/lkml/20240730-target-json-arrays-v1-1-2b376fd0ecf4@google.com/ [2] https://lore.kernel.org/all/20240813224027.84503-1-andrey.konovalov@linux.dev [3] https://lore.kernel.org/all/20240819213534.4080408-1-mmaurer@google.com/ Matthew Maurer (4): kbuild: rust: Define probing macros for rustc rust: kasan: Rust does not support KHWASAN kbuild: rust: Enable KASAN support kasan: rust: Add KASAN smoke test via UAF init/Kconfig | 1 + mm/kasan/Makefile | 7 ++- mm/kasan/kasan.h | 6 +++ mm/kasan/{kasan_test.c => kasan_test_c.c} | 12 +++++ mm/kasan/kasan_test_rust.rs | 19 ++++++++ scripts/Kconfig.include | 8 ++++ scripts/Makefile.compiler | 15 ++++++ scripts/Makefile.kasan | 57 ++++++++++++++++------- scripts/Makefile.lib | 3 ++ scripts/generate_rust_target.rs | 1 + 10 files changed, 112 insertions(+), 17 deletions(-) rename mm/kasan/{kasan_test.c => kasan_test_c.c} (99%) create mode 100644 mm/kasan/kasan_test_rust.rs