On Mon, Oct 21, 2024 at 7:58 PM Casey Schaufler [off-list ref] wrote:

On 10/21/2024 4:39 PM, Paul Moore wrote:

quoted

On Oct 14, 2024 Casey Schaufler [off-list ref] wrote:

quoted

Add a new lsm_context data structure to hold all the information about a
"security context", including the string, its size and which LSM allocated
the string. The allocation information is necessary because LSMs have
different policies regarding the lifecycle of these strings. SELinux
allocates and destroys them on each use, whereas Smack provides a pointer
to an entry in a list that never goes away.

Update security_release_secctx() to use the lsm_context instead of a
(char *, len) pair. Change its callers to do likewise.  The LSMs
supporting this hook have had comments added to remind the developer
that there is more work to be done.

The BPF security module provides all LSM hooks. While there has yet to
be a known instance of a BPF configuration that uses security contexts,
the possibility is real. In the existing implementation there is
potential for multiple frees in that case.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-integrity@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: audit@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: linux-nfs@vger.kernel.org
Cc: Todd Kjos <redacted>
Reviewed-by: Serge Hallyn <sergeh@kernel.org>
---
 drivers/android/binder.c                | 24 ++++++-------
 fs/ceph/xattr.c                         |  6 +++-
 fs/nfs/nfs4proc.c                       |  8 +++--
 fs/nfsd/nfs4xdr.c                       |  8 +++--
 include/linux/lsm_hook_defs.h           |  2 +-
 include/linux/security.h                | 35 +++++++++++++++++--
 include/net/scm.h                       | 11 +++---
 kernel/audit.c                          | 30 ++++++++---------
 kernel/auditsc.c                        | 23 +++++++------
 net/ipv4/ip_sockglue.c                  | 10 +++---
 net/netfilter/nf_conntrack_netlink.c    | 10 +++---
 net/netfilter/nf_conntrack_standalone.c |  9 +++--
 net/netfilter/nfnetlink_queue.c         | 13 ++++---
 net/netlabel/netlabel_unlabeled.c       | 45 +++++++++++--------------
 net/netlabel/netlabel_user.c            | 11 +++---
 security/apparmor/include/secid.h       |  2 +-
 security/apparmor/secid.c               | 11 ++++--
 security/security.c                     |  8 ++---
 security/selinux/hooks.c                | 11 ++++--
 19 files changed, 167 insertions(+), 110 deletions(-)

..

quoted

diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 1bc2d0890a9f..8303bbcfc543 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c

@@ -1127,14 +1122,14 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
             secid = addr6->secid;
     }

-    ret_val = security_secid_to_secctx(secid, &secctx, &secctx_len);
+    ret_val = security_secid_to_secctx(secid, &ctx.context, &ctx.len);
     if (ret_val != 0)
             goto list_cb_failure;
     ret_val = nla_put(cb_arg->skb,
                       NLBL_UNLABEL_A_SECCTX,
-                      secctx_len,
-                      secctx);
-    security_release_secctx(secctx, secctx_len);
+                      ctx.len,
+                      ctx.context);

Nitpicky alignment issue; please keep the arguments aligned as they
are currently.

Not a problem, although it looks like it's correct to me. I'll check to make sure.

Thanks.  It's likely just an oddity due to tabs rendering a bit odd in
the diff, I usually check that but maybe I didn't/forgot here.  Not a
major problem either way, I only mentioned it because I was commenting
on other patches in the series.

-- 
paul-moore.com