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

[Casey Schaufler <casey@schaufler-ca.com>]

On 3/3/2025 10:40 AM, Alice Ryhl wrote:
> On Mon, Mar 3, 2025 at 6:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 3/3/2025 7:29 AM, Alice Ryhl 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@google.com>
>>> ---
>>>  rust/kernel/security.rs | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> 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
>> I don't speak rust (well, yet?) so I can't talk about that, but this comment
>> has me concerned. Security contexts (secctx) are not destroyed, they are released.
>> While SELinux allocates and frees them, Smack maintains a list of contexts that
>> is never freed. A call to security_release_secctx() on SELinux "destroys" the
>> secctx, but for Smack does not.
> It's just a comment on a call to security_release_secctx, I can reword
> from "destroy" to "release".

That would do nicely. Thank you.

>
> Here's the full context:
>
> // 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 `security_release_secctx`.
> unsafe { bindings::security_release_secctx(&mut self.ctx) };
>
> Alice
>