[PATCH] lsm,nfs: fix memory leak of lsm_context

[Paul Moore]

On Thu, Feb 20, 2025 at 2:35 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
>
> On 2/20/2025 11:29 AM, Stephen Smalley wrote:
> > commit b530104f50e8 ("lsm: lsm_context in security_dentry_init_security")
> > did not preserve the lsm id for subsequent release calls, which results
> > in a memory leak. Fix it by saving the lsm id in the nfs4_label and
> > providing it on the subsequent release call.
> >
> > Fixes: b530104f50e8 ("lsm: lsm_context in security_dentry_init_security")
> > Signed-off-by: Stephen Smalley <stephen.smalley.work at gmail.com>
>
> Please don't accept this. I will have a better, simpler, more appropriate
> fix promptly.

We're still only at -rc3 so if you need another couple of days to get
an alternate solution out for review/discussion I think that's okay.
However, the solution proposed here is rather small and easy to
understand, which has a lot of value for an -rcX fix.  If the
alternative is notably larger and/or more complex, the patch here may
be preferable for v6.14 with the alternative/"proper-fix" being
something we can queue up for the next merge window.

Regardless, let's see Casey's fix before we worry too much about this.

> > ---
> >  fs/nfs/nfs4proc.c    | 7 ++++---
> >  include/linux/nfs4.h | 1 +
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index df9669d4ded7..c0caaec7bd20 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -133,6 +133,7 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> >       if (err)
> >               return NULL;
> >
> > +     label->lsmid = shim.id;
> >       label->label = shim.context;
> >       label->len = shim.len;
> >       return label;
> > @@ -145,7 +146,7 @@ nfs4_label_release_security(struct nfs4_label *label)
> >       if (label) {
> >               shim.context = label->label;
> >               shim.len = label->len;
> > -             shim.id = LSM_ID_UNDEF;
> > +             shim.id = label->lsmid;
> >               security_release_secctx(&shim);
> >       }
> >  }
> > @@ -6269,7 +6270,7 @@ static int _nfs4_get_security_label(struct inode *inode, void *buf,
> >                                       size_t buflen)
> >  {
> >       struct nfs_server *server = NFS_SERVER(inode);
> > -     struct nfs4_label label = {0, 0, buflen, buf};
> > +     struct nfs4_label label = {0, 0, 0, buflen, buf};
> >
> >       u32 bitmask[3] = { 0, 0, FATTR4_WORD2_SECURITY_LABEL };
> >       struct nfs_fattr fattr = {
> > @@ -6374,7 +6375,7 @@ static int nfs4_do_set_security_label(struct inode *inode,
> >  static int
> >  nfs4_set_security_label(struct inode *inode, const void *buf, size_t buflen)
> >  {
> > -     struct nfs4_label ilabel = {0, 0, buflen, (char *)buf };
> > +     struct nfs4_label ilabel = {0, 0, 0, buflen, (char *)buf };
> >       struct nfs_fattr *fattr;
> >       int status;
> >
> > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> > index 71fbebfa43c7..9ac83ca88326 100644
> > --- a/include/linux/nfs4.h
> > +++ b/include/linux/nfs4.h
> > @@ -47,6 +47,7 @@ struct nfs4_acl {
> >  struct nfs4_label {
> >       uint32_t        lfs;
> >       uint32_t        pi;
> > +     u32             lsmid;
> >       u32             len;
> >       char    *label;
> >  };

-- 
paul-moore.com