linux-kernel - Re: [PATCH v2 4/6] LSM: lsm_context in security_dentry_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d2d34843-e23c-40a7-92ae-5ebd7c678ad4@schaufler-ca.com>
Date: Mon, 21 Oct 2024 17:00:30 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Paul Moore <paul@...l-moore.com>, linux-security-module@...r.kernel.org
Cc: jmorris@...ei.org, serge@...lyn.com, keescook@...omium.org,
 john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
 stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org,
 selinux@...r.kernel.org, mic@...ikod.net, ceph-devel@...r.kernel.org,
 linux-nfs@...r.kernel.org, Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v2 4/6] LSM: lsm_context in security_dentry_init_security

On 10/21/2024 4:39 PM, Paul Moore wrote:
> On Oct 14, 2024 Casey Schaufler <casey@...aufler-ca.com> wrote:
>> Replace the (secctx,seclen) pointer pair with a single lsm_context
>> pointer to allow return of the LSM identifier along with the context
>> and context length. This allows security_release_secctx() to know how
>> to release the context. Callers have been modified to use or save the
>> returned data from the new structure.
>>
>> Special care is taken in the NFS code, which uses the same data structure
>> for its own copied labels as it does for the data which comes from
>> security_dentry_init_security().  In the case of copied labels the data
>> has to be freed, not released.
>>
>> The scaffolding funtion lsmcontext_init() is no longer needed and is
>> removed.
>>
>> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
>> Cc: ceph-devel@...r.kernel.org
>> Cc: linux-nfs@...r.kernel.org
>> ---
>>  fs/ceph/super.h               |  3 +--
>>  fs/ceph/xattr.c               | 16 ++++++----------
>>  fs/fuse/dir.c                 | 35 ++++++++++++++++++-----------------
>>  fs/nfs/dir.c                  |  2 +-
>>  fs/nfs/inode.c                | 17 ++++++++++-------
>>  fs/nfs/internal.h             |  8 +++++---
>>  fs/nfs/nfs4proc.c             | 22 +++++++++-------------
>>  fs/nfs/nfs4xdr.c              | 22 ++++++++++++----------
>>  include/linux/lsm_hook_defs.h |  2 +-
>>  include/linux/nfs4.h          |  8 ++++----
>>  include/linux/nfs_fs.h        |  2 +-
>>  include/linux/security.h      | 26 +++-----------------------
>>  security/security.c           |  9 ++++-----
>>  security/selinux/hooks.c      |  9 +++++----
>>  14 files changed, 80 insertions(+), 101 deletions(-)
> ..
>
>> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
>> index 039898d70954..47652d217d05 100644
>> --- a/include/linux/nfs_fs.h
>> +++ b/include/linux/nfs_fs.h
>> @@ -457,7 +457,7 @@ static inline void nfs4_label_free(struct nfs4_label *label)
>>  {
>>  #ifdef CONFIG_NFS_V4_SECURITY_LABEL
>>  	if (label) {
>> -		kfree(label->label);
>> +		kfree(label->lsmctx.context);
> Shouldn't this be a call to security_release_secctx() instead of a raw
> kfree()?

As mentioned in the description, the NFS data is a copy that NFS
manages, so it does need to be freed, not released.

>
>>  		kfree(label);
>>  	}
>>  #endif
> --
> paul-moore.com
>