Skip to content

new restriction lint: pointer_format #14792

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented May 13, 2025

I read a blog post about kernel security, and how various features might get lost while porting to Rust. In kernel C, they have some guardrails against divulging pointers. An easy way to replicate that in Rust is a lint for pointer formatting. So that's what this lint does.


changelog: new [pointer_format] lint

@rustbot
Copy link
Collaborator

rustbot commented May 13, 2025

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 13, 2025
@samueltardieu
Copy link
Contributor

Lint example needs to be fixed. FCP thread created on Zulip.

@rustbot label +S-final-comment-period

@rustbot rustbot added the S-final-comment-period Status: final comment period it will be merged unless new objections are raised (~1 week) label May 13, 2025
@tgross35
Copy link
Contributor

Is there any chance this could be adapted to check for any fmt trait being used on a pointer, rather than just the :p formatting? @nbdd0121 pointed out that a likely accident is printing via Debug, possibly through derives.

@llogiq
Copy link
Contributor Author

llogiq commented May 13, 2025

Do you mean debug-formatting a function pointer? That should be workable for the direct case but might become pretty hard for the general case.

@tgross35
Copy link
Contributor

I think a specific example would be something like this:

#[derive(Debug)]
pub struct S {
    pub p: *const u32,
}

pub fn foo(a: *const u32) {
    println!("{a:?}");
}

pub fn bar(a: S) {
    println!("{a:?}");
}

fn main() {
    foo(&0u32);
    bar(S{p: &0u32});
}

I'm not sure how this would be done in clippy since it would need to check whether a crate calls the Pointer / Debug implementations for raw pointers in the crate. If this isn't easy then checking directly in the format string still does help.

@llogiq
Copy link
Contributor Author

llogiq commented May 13, 2025

A thing we might do there is to walk the types of the arguments (as far as they derive Debug, we should not go into manual implementations) and see if there's any raw pointer or reference.

Comment on lines +202 to +203
/// In kernel context, this might be vulnerable to misuse for exfiltrating
/// stack or kernel function addresses.
Copy link
Contributor

@kpreid kpreid May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“…misuse for exfiltrating…” makes it sound like this is protection against attempts to insert malicious code into a project. Is auditing such code within scope for clippy? If not, it might be good to change the wording.

Also, “kernel context” isn’t very clear to people not familiar with what it’s talking about and sounds like it might be a formal or common Rust term, which it is not. How about something more like “In (projects|codebases|crates) such as operating system kernels,”?

@llogiq
Copy link
Contributor Author

llogiq commented May 16, 2025

Yeah, I'm working on some code to add that. I may not be able to look through manual Debug impls, though, so it's a best-effort thing at best.

@ojeda
Copy link
Contributor

ojeda commented May 18, 2025

Ideally, there would be a way to customize all pointer printing, i.e. including :p and :?, so that we can get them hashed as expected, and thus at that point we wouldn't need to lint for it. Meanwhile, this lint can be useful (and may be for other projects that do not want any printing whatsoever, too).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-final-comment-period Status: final comment period it will be merged unless new objections are raised (~1 week) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants