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

rust: add debugfs abstraction #1041

Draft
wants to merge 18 commits into
base: rust-next
Choose a base branch
from

Conversation

Fabo
Copy link

@Fabo Fabo commented Nov 14, 2023

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.

@tgross35
Copy link
Collaborator

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 Fabo force-pushed the fparent/rust-debugfs branch 3 times, most recently from 4d9ccdf to fb6844e Compare December 7, 2023 22:03
Darksonn and others added 18 commits December 14, 2023 11:23
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 <aliceryhl@google.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20230710074642.683831-1-aliceryhl@google.com
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 <Jamie.Cunliffe@arm.com>
Link: https://lore.kernel.org/r/20231020155056.3495121-2-Jamie.Cunliffe@arm.com
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 <Jamie.Cunliffe@arm.com>
Acked-by: Will Deacon <will@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Link: https://lore.kernel.org/r/20231020155056.3495121-3-Jamie.Cunliffe@arm.com
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 <ojeda@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Link: https://lore.kernel.org/r/20231214092958.377061-1-ojeda@kernel.org
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 <wedsonaf@gmail.com>
Co-developed-by: Daniel Xu <dxu@dxuuu.xyz>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-1-af617c0d9d94@google.com
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 <wedsonaf@gmail.com>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-2-af617c0d9d94@google.com
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 <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-3-af617c0d9d94@google.com
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 <wedsonaf@gmail.com>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-4-af617c0d9d94@google.com
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 <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-5-af617c0d9d94@google.com
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 <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-6-af617c0d9d94@google.com
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 <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231206-alice-file-v2-7-af617c0d9d94@google.com
[ 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 <daniel@iogearbox.net>
Link: llvm/llvm-project@c58f216 [1]
Link: https://godbolt.org/z/esT8xnaxj [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
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 <ojeda@kernel.org>
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 <ojeda@kernel.org>
Acked-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
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 <ojeda@kernel.org>
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 <fabien.parent@linaro.org>
Signed-off-by: Fabien Parent <fabien.parent@linaro.org>
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 <fabien.parent@linaro.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

7 participants