rust: add debugfs abstraction #1041
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
In case you haven't seen it, there is another debugfs implementation #885. Not sure how far along that is but you could likely build some off of it |
Fabo
force-pushed
the
fparent/rust-debugfs
branch
3 times, most recently
from
4d9ccdf
to
fb6844e
Compare
Previously, the `ForeignOwnable` trait had a method called `borrow_mut` that was intended to provide mutable access to the inner value. However, the method accidentally made it possible to change the address of the object being modified, which usually isn't what we want. (And when we want that, it can be done by calling `from_foreign` and `into_foreign`, like how the old `borrow_mut` was implemented.) In this patch, we introduce an alternate definition of `borrow_mut` that solves the previous problem. Conceptually, given a pointer type `P` that implements `ForeignOwnable`, the `borrow_mut` method gives you the same kind of access as an `&mut P` would, except that it does not let you change the pointer `P` itself. This is analogous to how the existing `borrow` method provides the same kind of access to the inner value as an `&P`. Note that for types like `Arc`, having an `&mut Arc<T>` only gives you immutable access to the inner `T`. This is because mutable references assume exclusive access, but there might be other handles to the same reference counted value, so the access isn't exclusive. The `Arc` type implements this by making `borrow_mut` return the same type as `borrow`. Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Benno Lossin <[email protected]> Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Boqun Feng <[email protected]> Link: https://lore.kernel.org/r/[email protected]
Eventually we want all architectures to be using the target as defined by rustc. However currently some architectures can't do that and are using the target.json specification. This puts in place the foundation to allow the use of the builtin target definition or a target.json specification. Signed-off-by: Jamie Cunliffe <[email protected]> Link: https://lore.kernel.org/r/[email protected]
This commit provides the build flags for Rust for AArch64. The core Rust support already in the kernel does the rest. This enables the PAC ret and BTI options in the Rust build flags to match the options that are used when building C. The Rust samples have been tested with this commit. Signed-off-by: Jamie Cunliffe <[email protected]> Acked-by: Will Deacon <[email protected]> Acked-by: Catalin Marinas <[email protected]> Link: https://lore.kernel.org/r/[email protected]
This is the next upgrade to the Rust toolchain, from 1.73.0 to 1.74.1 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features No unstable features (that we use) were stabilized. Therefore, the only unstable features allowed to be used outside the `kernel` crate are still `new_uninit,offset_of`, though other code to be upstreamed may increase the list (e.g. `offset_of` was added recently). Please see [3] for details. # Other improvements Rust 1.74.0 allows to use `#[repr(Rust)]` explicitly [4], which can be useful to be explicit about particular cases that would normally use e.g. the C representation, such as silencing lints like the upcoming additions we requested [5] to the `no_mangle_with_rust_abi` Clippy lint (which in turn triggered the `#[repr(Rust)]` addition). Rust 1.74.0 includes a fix for one of the false negative cases we reported in Clippy's `disallowed_macros` lint [6] that we would like to use in the future. Rust 1.74.1 fixes an ICE that the Apple AGX GPU driver was hitting [7]. # Required changes For this upgrade, no changes were required (i.e. on our side). # `alloc` upgrade and reviewing The vast majority of changes are due to our `alloc` fork being upgraded at once. There are two kinds of changes to be aware of: the ones coming from upstream, which we should follow as closely as possible, and the updates needed in our added fallible APIs to keep them matching the newer infallible APIs coming from upstream. Instead of taking a look at the diff of this patch, an alternative approach is reviewing a diff of the changes between upstream `alloc` and the kernel's. This allows to easily inspect the kernel additions only, especially to check if the fallible methods we already have still match the infallible ones in the new version coming from upstream. Another approach is reviewing the changes introduced in the additions in the kernel fork between the two versions. This is useful to spot potentially unintended changes to our additions. To apply these approaches, one may follow steps similar to the following to generate a pair of patches that show the differences between upstream Rust and the kernel (for the subset of `alloc` we use) before and after applying this patch: # Get the difference with respect to the old version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > old.patch git -C linux restore rust/alloc # Apply this patch. git -C linux am rust-upgrade.patch # Get the difference with respect to the new version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > new.patch git -C linux restore rust/alloc Now one may check the `new.patch` to take a look at the additions (first approach) or at the difference between those two patches (second approach). For the latter, a side-by-side tool is recommended. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1741-2023-12-07 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: Rust-for-Linux#2 [3] Link: rust-lang/rust#114201 [4] Link: rust-lang/rust-clippy#11219 [5] Link: rust-lang/rust-clippy#11431 [6] Link: rust-lang/rust#117976 (comment) [7] Signed-off-by: Miguel Ojeda <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Link: https://lore.kernel.org/r/[email protected]
This abstraction makes it possible to manipulate the open files for a process. The new `File` struct wraps the C `struct file`. When accessing it using the smart pointer `ARef<File>`, the pointer will own a reference count to the file. When accessing it as `&File`, then the reference does not own a refcount, but the borrow checker will ensure that the reference count does not hit zero while the `&File` is live. Since this is intended to manipulate the open files of a process, we introduce a `from_fd` constructor that corresponds to the C `fget` method. In future patches, it will become possible to create a new fd in a process and bind it to a `File`. Rust Binder will use these to send fds from one process to another. We also provide a method for accessing the file's flags. Rust Binder will use this to access the flags of the Binder fd to check whether the non-blocking flag is set, which affects what the Binder ioctl does. This introduces a struct for the EBADF error type, rather than just using the Error type directly. This has two advantages: * `File::from_fd` returns a `Result<ARef<File>, BadFdError>`, which the compiler will represent as a single pointer, with null being an error. This is possible because the compiler understands that `BadFdError` has only one possible value, and it also understands that the `ARef<File>` smart pointer is guaranteed non-null. * Additionally, we promise to users of the method that the method can only fail with EBADF, which means that they can rely on this promise without having to inspect its implementation. That said, there are also two disadvantages: * Defining additional error types involves boilerplate. * The question mark operator will only utilize the `From` trait once, which prevents you from using the question mark operator on `BadFdError` in methods that return some third error type that the kernel `Error` is convertible into. (However, it works fine in methods that return `Error`.) Signed-off-by: Wedson Almeida Filho <[email protected]> Co-developed-by: Daniel Xu <[email protected]> Signed-off-by: Daniel Xu <[email protected]> Co-developed-by: Alice Ryhl <[email protected]> Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
Add a wrapper around `struct cred` called `Credential`, and provide functionality to get the `Credential` associated with a `File`. Rust Binder must check the credentials of processes when they attempt to perform various operations, and these checks usually take a `&Credential` as parameter. The security_binder_set_context_mgr function would be one example. This patch is necessary to access these security_* methods from Rust. Signed-off-by: Wedson Almeida Filho <[email protected]> Co-developed-by: Alice Ryhl <[email protected]> Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
Adds an abstraction for viewing the string representation of a security context. This is needed by Rust Binder because it has feature where a process can view the string representation of the security context for incoming transactions. The process can use that to authenticate incoming transactions, and since the feature is provided by the kernel, the process can trust that the security context is legitimate. Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
Allow for the creation of a file descriptor in two steps: first, we reserve a slot for it, then we commit or drop the reservation. The first step may fail (e.g., the current process ran out of available slots), but commit and drop never fail (and are mutually exclusive). This is needed by Rust Binder when fds are sent from one process to another. It has to be a two-step process to properly handle the case where multiple fds are sent: The operation must fail or succeed atomically, which we achieve by first reserving the fds we need, and only installing the files once we have reserved enough fds to send the files. Fd reservations assume that the value of `current` does not change between the call to get_unused_fd_flags and the call to fd_install (or put_unused_fd). By not implementing the Send trait, this abstraction ensures that the `FileDescriptorReservation` cannot be moved into a different process. Signed-off-by: Wedson Almeida Filho <[email protected]> Co-developed-by: Alice Ryhl <[email protected]> Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
Adds a wrapper around `kuid_t` called `Kuid`. This allows us to define various operations on kuids such as equality and current_euid. It also lets us provide conversions from kuid into userspace values. Rust Binder needs these operations because it needs to compare kuids for equality, and it needs to tell userspace about the pid and uid of incoming transactions. Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
To close an fd from kernel space, we could call `ksys_close`. However, if we do this to an fd that is held using `fdget`, then we may trigger a use-after-free. Introduce a helper that can be used to close an fd even if the fd is currently held with `fdget`. This is done by grabbing an extra refcount to the file and dropping it in a task work once we return to userspace. This is necessary for Rust Binder because otherwise the user might try to have Binder close its fd for /dev/binder, which would cause problems as this happens inside an ioctl on /dev/binder, and ioctls hold the fd using `fdget`. Additional motivation can be found in commit 80cd795 ("binder: fix use-after-free due to ksys_close() during fdget()") and in the comments on `binder_do_fd_close`. If there is some way to detect whether an fd is currently held with `fdget`, then this could be optimized to skip the allocation and task work when this is not the case. Another possible optimization would be to combine several fds into a single task work, since this is used with fd arrays that might hold several fds. That said, it might not be necessary to optimize it, because Rust Binder has two ways to send fds: BINDER_TYPE_FD and BINDER_TYPE_FDA. With BINDER_TYPE_FD, it is userspace's responsibility to close the fd, so this mechanism is used only by BINDER_TYPE_FDA, but fd arrays are used rarely these days. Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
The existing `CondVar` abstraction is a wrapper around `wait_list`, but it does not support all use-cases of the C `wait_list` type. To be specific, a `CondVar` cannot be registered with a `struct poll_table`. This limitation has the advantage that you do not need to call `synchronize_rcu` when destroying a `CondVar`. However, we need the ability to register a `poll_table` with a `wait_list` in Rust Binder. To enable this, introduce a type called `PollCondVar`, which is like `CondVar` except that you can register a `poll_table`. We also introduce `PollTable`, which is a safe wrapper around `poll_table` that is intended to be used with `PollCondVar`. The destructor of `PollCondVar` unconditionally calls `synchronize_rcu` to ensure that the removal of epoll waiters has fully completed before the `wait_list` is destroyed. That said, `synchronize_rcu` is rather expensive and is not needed in all cases: If we have never registered a `poll_table` with the `wait_list`, then we don't need to call `synchronize_rcu`. (And this is a common case in Binder - not all processes use Binder with epoll.) The current implementation does not account for this, but if we find that it is necessary to improve this, a future patch could change store a boolean next to the `wait_list` to keep track of whether a `poll_table` has ever been registered. Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected] [ boqun: Removes unused POLLFREE definition ]
Support the `RETPOLINE` speculation mitigation by enabling the target
features that Clang does.
The existing target feature being enabled was a leftover from
our old `rust` branch, and it is not enough: the target feature
`retpoline-external-thunk` only implies `retpoline-indirect-calls`, but
not `retpoline-indirect-branches` (see LLVM's `X86.td`), unlike Clang's
flag of the same name `-mretpoline-external-thunk` which does imply both
(see Clang's `lib/Driver/ToolChains/Arch/X86.cpp`).
Without this, `objtool` would complain if enabled for individual object
files (like it is planned in the future), e.g.
rust/core.o: warning: objtool:
_R...escape_default+0x13: indirect jump found in RETPOLINE build
In addition, change the comment to note that LLVM is the one disabling
jump tables when retpoline is enabled, thus we do not need to use
`-Zno-jump-tables` for Rust here -- see commit c58f2166ab39 ("Introduce
the "retpoline" x86 mitigation technique ...") [1]:
The goal is simple: avoid generating code which contains an indirect
branch that could have its prediction poisoned by an attacker. In
many cases, the compiler can simply use directed conditional
branches and a small search tree. LLVM already has support for
lowering switches in this way and the first step of this patch is
to disable jump-table lowering of switches and introduce a pass to
rewrite explicit indirectbr sequences into a switch over integers.
As well as a live example at [2].
Cc: Daniel Borkmann <[email protected]>
Link: llvm/llvm-project@c58f216 [1]
Link: https://godbolt.org/z/esT8xnaxj [2]
Signed-off-by: Miguel Ojeda <[email protected]>
Support the `SLS` speculation mitigation by enabling the target features
that Clang does.
Without this, `objtool` would complain if enabled for individual object
files (like it is planned in the future), e.g.
rust/core.o: warning: objtool:
_R...next_up+0x44: missing int3 after ret
Signed-off-by: Miguel Ojeda <[email protected]>
The Rust compiler does not support the equivalent of
`-mfunction-return=thunk-extern` yet [1]. Thus, currently, `objtool`
warns about it, e.g.:
samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c:
'naked' return found in RETHUNK build
The support in `rustc` for `-Zfunction-return` has been submitted and
is being reviewed [2]. It adds the needed LLVM function attributes and,
with it, I got a RETHUNK kernel build with Rust enabled that does not
print the `objtool` related warnings, boots in QEMU and can load a kernel
loadable module.
In any case, until proper/complete support is added to `rustc`, make it
a hard restriction until the mitigation is in place.
This may have an impact for developers that may not need/care about the
mitigation in the Rust side (e.g. Ubuntu offers Rust as a "technology
preview" [3]), but given we are getting closer to having the first actual
in-tree Rust kernel users, it seems like the right time to disallow
it. This should also avoid confusion [4].
Link: rust-lang/rust#116853 [1]
Link: rust-lang/rust#116892 [2]
Link: https://lore.kernel.org/rust-for-linux/ZSQXqX2%2Flhf5ICZP@gpd/ [3]
Link: https://lore.kernel.org/rust-for-linux/CANiq72n6DMeXQrgOzS_+3VdgNYAmpcnneAHJnZERUQhMExg+0A@mail.gmail.com/ [4]
Signed-off-by: Miguel Ojeda <[email protected]>
Acked-by: Martin Rodriguez Reboredo <[email protected]>
When support for `-Zfunction-return` lands in Rust [1], this patch may be used to enable RETHUNK support on top of the previous patch. Link: rust-lang/rust#116892 [1] Signed-off-by: Miguel Ojeda <[email protected]>
Port file::Operations and its dependencies from the `rust` branch. This is a trimmed version from what can be found in `rust` since I only need a subset of the functionality. Signed-off-by: Fabien Parent <[email protected]>
Signed-off-by: Fabien Parent <[email protected]>
Add some basic debugfs sample module to show how to use the debugfs API.
The sample module is showing:
* How to create a directory
* How to define and register a debugfs file by implementing
file::Operations
* How to use the attribute macro to expose simple numerical debugfs
values. This is the equivalent of the C macro
DEFINE_DEBUGFS_ATTRIBUTE{,_SIGNED}
Signed-off-by: Fabien Parent <[email protected]>
Fabo
force-pushed
the
fparent/rust-debugfs
branch
from
fb6844e
to
e05f067
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a driver, I have working on a debugfs abstraction.
This is not considered finished, and there are many things I want to change, so no need to review it yet. But anyway I decided to share it here for visibility and in case anyone else needs it.
This driver will depend on file::Operations to be upsteamed.