Add `kernel-address` sanitizer support for freestanding targets #99679

repnop · 2022-07-24T15:23:30Z

This PR adds support for KASan (kernel address sanitizer) instrumentation in freestanding targets. I included the minimal set of x86_64-unknown-none, riscv64{imac, gc}-unknown-none-elf, and aarch64-unknown-none but there's likely other targets it can be added to. (linux_kernel_base.rs?) KASan uses the address sanitizer attributes but has the CompileKernel parameter set to true in the pass creation.

rust-highfive · 2022-07-24T15:23:33Z

r? @cuviper

(rust-highfive has picked a reviewer for you, use r? to override)

rust-highfive · 2022-07-24T15:23:34Z

⚠️ Warning ⚠️

These commits modify compiler targets. (See the Target Tier Policy.)

Andy-Python-Programmer · 2022-07-24T22:02:45Z

Amazing! 🎉

cuviper

Are there any tests we can add for this? Perhaps add to the sanitizer tests in src/test/codegen/?

How about some documentation in src/doc/unstable-book/src/compiler-flags/sanitizer.md?

Should this be added to Session::emit_lifetime_markers?

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

compiler/rustc_session/src/options.rs

cuviper · 2022-07-28T23:34:57Z

but there's likely other targets it can be added to. (linux_kernel_base.rs?)

That sounds reasonable -- is there any reason not to add it there?

repnop · 2022-08-02T23:11:45Z

Are there any tests we can add for this? Perhaps add to the sanitizer tests in src/test/codegen/?

it uses the same instrumentation as Address Sanitizer so I think it should be pretty simple to integrate with those, I'll take a look!

How about some documentation in src/doc/unstable-book/src/compiler-flags/sanitizer.md?

didn't even think of that! 😅 good catch

Should this be added to Session::emit_lifetime_markers?

hmm, I'm not familiar with that area, but I'll see what I can find out!

but there's likely other targets it can be added to. (linux_kernel_base.rs?)

That sounds reasonable -- is there any reason not to add it there?

I thought so too, but I can't say I'm 100% confident if it should be. @alex you added the original x86_64-linux-kernel target, do you know if there'd be any issues with enabling KASan for it? my thought would be that since it needs explicit kernel support, it would probably only make sense if the kernel its running on has KASan enabled?

as for other targets, I was hoping to get some feedback from someone more familiar with the set of targets that would actually support this -- I'm mainly unsure of the various bare-metal ARM targets, but I'll also see if I can dig up what clang supports the flag on and use that as a reference 👍

bors · 2022-09-01T06:10:06Z

☔ The latest upstream changes (presumably #100537) made this pull request unmergeable. Please resolve the merge conflicts.

pnkfelix · 2022-09-08T14:36:42Z

based on the previous comment from @repnop, I assume this is waiting on @repnop to integrate the review feedback provided by @cuviper

@rustbot label: -S-waiting-on-review S-waiting-on-author

repnop · 2022-09-12T01:05:14Z

@cuviper I believe all of the changes you requested are now done. I split out the tests where appropriate since the targets with the other sanitizers won't support this. I ran both of them locally with ./x.py test --force-rerun src/test/ui/sanitize/cfg-kasan.rs src/test/codegen/sanitizer-kasan-emits-instrumentation.rs --target=riscv64imac-unknown-none-elf --host=x86_64-unknown-linux-gnu and got a pass for each, so I think things are looking good there. I did find out that I had to modify the cfg setting code because KASAN is really just a different ASAN compilation method, so I figure the attributes should be the same for both.

As for the x86_64-unknown-none-linuxkernel target that I had mentioned previously, I dug around and I cannot find a single project in the wild except for one in a google search for that target that hasn't been updated in 2 years, the Rust for Linux uses a custom target.json for their builds with a different LLVM target, and as far as I can tell, its nigh identical to the x86_64-unknown-none target, so I'm going to ignore it now as I don't think its likely to exist for long unless there's a very good reason for it to.

repnop · 2022-09-13T22:56:05Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

bors · 2022-09-25T17:10:51Z

☔ The latest upstream changes (presumably #102265) made this pull request unmergeable. Please resolve the merge conflicts.

wesleywiser · 2022-10-06T14:37:18Z

Hey @cuviper, it looks like this PR is ready for another round of review. Thanks! 🙂

wesleywiser · 2022-10-06T14:39:06Z

@repnop would you mind rebasing and resolving the merge conflicts? That will help shorten the review cycle.

rustbot · 2023-01-29T15:58:38Z

These commits modify compiler targets.
(See the Target Tier Policy.)

bors · 2023-01-29T22:27:30Z

☔ The latest upstream changes (presumably #107451) made this pull request unmergeable. Please resolve the merge conflicts.

repnop · 2023-01-31T17:22:45Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

cuviper

Just a couple typos for riscv min-llvm-version.

tests/codegen/sanitizer-kasan-emits-instrumentation.rs

bors · 2023-02-15T00:03:34Z

☔ The latest upstream changes (presumably #108056) made this pull request unmergeable. Please resolve the merge conflicts.

cuviper

@bors r+

(bah, bors doesn't see reviews...)

cuviper · 2023-02-15T18:46:00Z

@bors r+

bors · 2023-02-15T18:46:02Z

📌 Commit 1971438 has been approved by cuviper

It is now in the queue for this repository.

…cuviper Add `kernel-address` sanitizer support for freestanding targets This PR adds support for KASan (kernel address sanitizer) instrumentation in freestanding targets. I included the minimal set of `x86_64-unknown-none`, `riscv64{imac, gc}-unknown-none-elf`, and `aarch64-unknown-none` but there's likely other targets it can be added to. (`linux_kernel_base.rs`?) KASan uses the address sanitizer attributes but has the `CompileKernel` parameter set to `true` in the pass creation.

bors · 2023-02-18T03:05:14Z

⌛ Testing commit 1971438 with merge fabfd1f...

bors · 2023-02-18T05:58:34Z

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing fabfd1f to master...

rust-timer · 2023-02-18T07:11:06Z

Finished benchmarking commit (fabfd1f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.1%, 0.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-3.4%, -3.4%]	2
All ❌✅ (primary)	0.1%	[0.1%, 0.1%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.2%	[4.3%, 5.9%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

rust-highfive assigned cuviper Jul 24, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 24, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2022

This comment has been minimized.

Sign in to view

repnop force-pushed the kernel-address-sanitizer branch from 52a015d to f564b48 Compare July 24, 2022 21:09

This comment has been minimized.

Sign in to view

repnop force-pushed the kernel-address-sanitizer branch from f564b48 to 4d3b130 Compare July 25, 2022 00:50

cuviper requested changes Jul 28, 2022

View reviewed changes

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp Outdated Show resolved Hide resolved

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp Outdated Show resolved Hide resolved

compiler/rustc_session/src/options.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2022

repnop force-pushed the kernel-address-sanitizer branch 2 times, most recently from c91f6c2 to b705dfb Compare September 12, 2022 01:00

repnop requested a review from cuviper September 12, 2022 01:06

This comment has been minimized.

Sign in to view

repnop force-pushed the kernel-address-sanitizer branch from b705dfb to b3db2d5 Compare September 12, 2022 22:34

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2022

repnop force-pushed the kernel-address-sanitizer branch from b3db2d5 to 98bbbc8 Compare October 6, 2022 23:35

rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Jan 29, 2023

This comment has been minimized.

Sign in to view

repnop force-pushed the kernel-address-sanitizer branch from fa5f1e5 to b59522e Compare January 29, 2023 17:15

This comment has been minimized.

Sign in to view

repnop force-pushed the kernel-address-sanitizer branch from b59522e to aac8575 Compare January 29, 2023 18:25

repnop force-pushed the kernel-address-sanitizer branch from aac8575 to 373aaa1 Compare January 31, 2023 02:03

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 31, 2023

cuviper requested changes Feb 8, 2023

View reviewed changes

tests/codegen/sanitizer-kasan-emits-instrumentation.rs Outdated Show resolved Hide resolved

tests/codegen/sanitizer-kasan-emits-instrumentation.rs Outdated Show resolved Hide resolved

repnop force-pushed the kernel-address-sanitizer branch from 373aaa1 to 3508161 Compare February 12, 2023 02:59

repnop requested a review from cuviper February 12, 2023 22:52

Add kernel-address sanitizer support for freestanding targets

1971438

repnop force-pushed the kernel-address-sanitizer branch from 3508161 to 1971438 Compare February 15, 2023 01:54

cuviper approved these changes Feb 15, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 15, 2023

ckxkexing mentioned this pull request Feb 17, 2023

Rollup of 3 pull requests #108153

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 18, 2023

bors merged commit fabfd1f into rust-lang:master Feb 18, 2023

rustbot added this to the 1.69.0 milestone Feb 18, 2023

Add kernel-address sanitizer support for freestanding targets #99679

Add kernel-address sanitizer support for freestanding targets #99679

Uh oh!

Conversation

repnop commented Jul 24, 2022

Uh oh!

rust-highfive commented Jul 24, 2022

Uh oh!

rust-highfive commented Jul 24, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

Andy-Python-Programmer commented Jul 24, 2022

Uh oh!

cuviper left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cuviper commented Jul 28, 2022

Uh oh!

repnop commented Aug 2, 2022

Uh oh!

bors commented Sep 1, 2022

Uh oh!

pnkfelix commented Sep 8, 2022

Uh oh!

repnop commented Sep 12, 2022

Uh oh!

This comment has been minimized.

repnop commented Sep 13, 2022

Uh oh!

bors commented Sep 25, 2022

Uh oh!

wesleywiser commented Oct 6, 2022

Uh oh!

wesleywiser commented Oct 6, 2022

Uh oh!

rustbot commented Jan 29, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented Jan 29, 2023

Uh oh!

repnop commented Jan 31, 2023

Uh oh!

cuviper left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bors commented Feb 15, 2023

Uh oh!

cuviper left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cuviper commented Feb 15, 2023

Uh oh!

bors commented Feb 15, 2023

Uh oh!

bors commented Feb 18, 2023

Uh oh!

bors commented Feb 18, 2023

Uh oh!

rust-timer commented Feb 18, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

Add `kernel-address` sanitizer support for freestanding targets #99679

Add `kernel-address` sanitizer support for freestanding targets #99679

cuviper left a comment •

edited

Loading