On Thu, Feb 20, 2025 at 2:30 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 7 ++++---
> include/linux/nfs4.h | 1 +
> 2 files changed, 5 insertions(+), 3 deletions(-)
Now that we've seen Casey's patch, I believe this patch is the better
solution and we should get this up to Linus during the v6.14-rcX
phase. I've got one minor nitpick (below), but how would you like to
handle this patch NFS folks? I'm guessing you would prefer to pull
this via the NFS tree, but if not let me know and I can pull it via
the LSM tree (an ACK would be appreciated).
Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> 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;
I don't think this is a significant problem, but considering that
lsm_context::id is an int, the lsmid field above should probably be an
int too.
> u32 len;
> char *label;
> };
> --
> 2.47.1
--
paul-moore.com
