Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No equivalent of -fno-delete-null-pointer-checks #1052

Open
comex opened this issue Dec 28, 2023 · 1 comment
Open

No equivalent of -fno-delete-null-pointer-checks #1052

comex opened this issue Dec 28, 2023 · 1 comment
Labels
• toolchain Related to `rustc`, `bindgen`, `rustdoc`, LLVM, Clippy...

Comments

@comex
Copy link

comex commented Dec 28, 2023

Suppose I add this (buggy) code to rust_minimal.rs:

/// Callers should ensure that `p` is either valid to dereference or null.
pub unsafe fn questionable(p: *const i32) -> i32 {
    // SAFETY: oops, accidentally dereferenced before checking
    let val = unsafe { *p };
    if p.is_null() {
        val
    } else {
        print_not_null();
        0
    }
}

#[inline(never)]
fn print_not_null() {
    pr_info!("The pointer was not null!");
}

When I build it, questionable is compiled into an unconditional call to print_not_null. The compiler assumes that p can't be null at the point it's checked, because if it were null, the program would have already executed undefined behavior when it dereferenced it.

C compilers perform the same kind of optimization by default, but Linux disables it with the compiler flag -fno-delete-null-pointer-checks. rustc has no equivalent option. Implementation-wise, I believe it would be straightforward to add one at least for rustc's LLVM backend, since rustc can do the same thing Clang does when passed -fno-delete-null-pointer-checks – namely, add the LLVM IR attribute null_pointer_is_valid to all generated code. Perhaps such an option should be added to the wishlist.

On the other hand, null pointer optimizations might not be the end of the world. They only affect code that executes undefined behavior, which is easier to avoid in Rust than in C.

Also, null dereferences usually trap in practice; it doesn't matter if a later null check is eliminated if the kernel can't ever reach that point. For a null dereference not to trap, not only does mmap_min_addr have to be 0, the CPU also has to allow kernel accesses to reach userland mappings. On x86 this is prevented by the decade-old SMAP extension, and on arm64 by the PAN extension.

However, compiler optimizations can defeat that reasoning. In the above example, once the null check is eliminated, val becomes unused, so the load from null is also eliminated. Thus no trap occurs at runtime; you just silently get 'incorrect' behavior (i.e. print_not_null is called even if the pointer is null). Admittedly, I designed the example specifically to achieve that effect, and real code is pretty unlikely to follow the same pattern. In more realistic cases such as loads that are completely unused, LLVM usually doesn't take advantage of the undefined behavior. So the current threat may not be that high. But that's really just a limitation of LLVM's optimizer, and you never know if it might get smarter in the future.

@nbdd0121
Copy link
Member

Personally, I don't think it's too important for Rust kernel code.

  • Option should allow us to avoid having nullable pointers at all.
  • Telling LLVM that null pointer is valid alone also likely not sufficient in the long term since that flag also needs to change Rust opsem, e.g. ensuring that MIR opts would also obey it. It might become a maintenance burden.
  • Niche optimisation can exploit such UB even when other code is told not to. E.g. if you dereference a pointer, and then turn it into Option<&>, then the compiler may think it's Some despite memory-representation-wise it's null. null_pointer_is_valid might only be useful if it also disables niche, but it's not doable ABI-wise.

@ojeda ojeda added the • toolchain Related to `rustc`, `bindgen`, `rustdoc`, LLVM, Clippy... label Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
• toolchain Related to `rustc`, `bindgen`, `rustdoc`, LLVM, Clippy...
Development

No branches or pull requests

3 participants