Re: [PATCH] lsm: rust: mark SecurityCtx methods inline

[Paul Moore]

On Mon, Mar 3, 2025 at 10:30 AM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
>
> I'm seeing Binder generating calls to methods on SecurityCtx such as
> from_secid and drop without inlining. Since these methods are really
> simple wrappers around C functions, mark the methods to inline to avoid
> generating these useless small functions.
>
> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> ---
>  rust/kernel/security.rs | 5 +++++
>  1 file changed, 5 insertions(+)

While this isn't specific to your patch, Casey's comment about "free"
vs "release" is something to keep in mind when working on the LSM
bindings; what happens inside the individual LSMs for a given LSM hook
can vary quite a bit.

I also saw a similar Rust patch where a commenter suggested using
"impersonal facts" in the commit description and I believe that is a
good idea here as well.

Beyond those nitpicks, this looks okay to me based on my *extremely*
limited Rust knowledge.  With the minor requested changes in place,
would you prefer me to take this via the LSM tree, or would you prefer
it to go up to Linus via a more Rust-y tree?

> diff --git a/rust/kernel/security.rs b/rust/kernel/security.rs
> index 25d2b1ac3833..243211050526 100644
> --- a/rust/kernel/security.rs
> +++ b/rust/kernel/security.rs
> @@ -23,6 +23,7 @@ pub struct SecurityCtx {
>
>  impl SecurityCtx {
>      /// Get the security context given its id.
> +    #[inline]
>      pub fn from_secid(secid: u32) -> Result<Self> {
>          // SAFETY: `struct lsm_context` can be initialized to all zeros.
>          let mut ctx: bindings::lsm_context = unsafe { core::mem::zeroed() };
> @@ -35,16 +36,19 @@ pub fn from_secid(secid: u32) -> Result<Self> {
>      }
>
>      /// Returns whether the security context is empty.
> +    #[inline]
>      pub fn is_empty(&self) -> bool {
>          self.ctx.len == 0
>      }
>
>      /// Returns the length of this security context.
> +    #[inline]
>      pub fn len(&self) -> usize {
>          self.ctx.len as usize
>      }
>
>      /// Returns the bytes for this security context.
> +    #[inline]
>      pub fn as_bytes(&self) -> &[u8] {
>          let ptr = self.ctx.context;
>          if ptr.is_null() {
> @@ -61,6 +65,7 @@ pub fn as_bytes(&self) -> &[u8] {
>  }
>
>  impl Drop for SecurityCtx {
> +    #[inline]
>      fn drop(&mut self) {
>          // SAFETY: By the invariant of `Self`, this frees a context that came from a successful
>          // call to `security_secid_to_secctx` and has not yet been destroyed by
>
> ---
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> change-id: 20250303-inline-securityctx-6fc1ca669156
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@xxxxxxxxxx>

-- 
paul-moore.com