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

Alice Ryhl
2025-03-03T18:40:53Z

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".

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