Op 23-10-2024 om 23:21 schreef Casey Schaufler:
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@xxxxxxxxxxxxxxxx>
Cc: linux-integrity@xxxxxxxxxxxxxxx
Cc: netdev@xxxxxxxxxxxxxxx
Cc: audit@xxxxxxxxxxxxxxx
Cc: netfilter-devel@xxxxxxxxxxxxxxx
To: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
Cc: linux-nfs@xxxxxxxxxxxxxxx
Cc: Todd Kjos <tkjos@xxxxxxxxxx>
---
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 | 40 +++++++++++--------------
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, 165 insertions(+), 107 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 978740537a1a..d4229bf6f789 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3011,8 +3011,7 @@ static void binder_transaction(struct binder_proc *proc,
struct binder_context *context = proc->context;
int t_debug_id = atomic_inc_return(&binder_last_id);
ktime_t t_start_time = ktime_get();
- char *secctx = NULL;
- u32 secctx_sz = 0;
+ struct lsm_context lsmctx;
Not initialized ?
struct list_head sgc_head;
struct list_head pf_head;
const void __user *user_buffer = (const void __user *)
@@ -3291,7 +3290,8 @@ static void binder_transaction(struct binder_proc *proc,
size_t added_size;
security_cred_getsecid(proc->cred, &secid);
- ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
+ ret = security_secid_to_secctx(secid, &lsmctx.context,
+ &lsmctx.len);
if (ret) {
binder_txn_error("%d:%d failed to get security context\n",
thread->pid, proc->pid);
@@ -3300,7 +3300,7 @@ static void binder_transaction(struct binder_proc *proc,
return_error_line = __LINE__;
goto err_get_secctx_failed;
}
- added_size = ALIGN(secctx_sz, sizeof(u64));
+ added_size = ALIGN(lsmctx.len, sizeof(u64));
extra_buffers_size += added_size;
if (extra_buffers_size < added_size) {
binder_txn_error("%d:%d integer overflow of extra_buffers_size\n",
@@ -3334,23 +3334,23 @@ static void binder_transaction(struct binder_proc *proc,
t->buffer = NULL;
goto err_binder_alloc_buf_failed;
}
- if (secctx) {
+ if (lsmctx.context) {
From code inspection it is not immediately obvious. Can you
guarantee that lsmctx is always initialized when the code
gets to this point? Perhaps it is safer to just initialize when
it is defined above (line 3014).
