[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zw3kG5-Xh4iqDYSz@lei>
Date: Tue, 15 Oct 2024 03:40:11 +0000
From: sergeh@...nel.org
To: Casey Schaufler <casey@...aufler-ca.com>
Cc: paul@...l-moore.com, linux-security-module@...r.kernel.org,
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,
linux-integrity@...r.kernel.org, netdev@...r.kernel.org,
audit@...r.kernel.org, netfilter-devel@...r.kernel.org,
linux-nfs@...r.kernel.org, Todd Kjos <tkjos@...gle.com>
Subject: Re: [PATCH v2 1/6] LSM: Ensure the correct LSM context releaser
On Mon, Oct 14, 2024 at 08:14:45AM -0700, Casey Schaufler wrote:
> 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@...aufler-ca.com>
Thanks.
Reviewed-by: Serge Hallyn <sergeh@...nel.org>
for the set (as far as affects lsm, of course).
> Cc: linux-integrity@...r.kernel.org
> Cc: netdev@...r.kernel.org
> Cc: audit@...r.kernel.org
> Cc: netfilter-devel@...r.kernel.org
> To: Pablo Neira Ayuso <pablo@...filter.org>
> Cc: linux-nfs@...r.kernel.org
> Cc: Todd Kjos <tkjos@...gle.com>
> ---
> 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(-)
>
> 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;
> 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) {
> int err;
> size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
> ALIGN(tr->offsets_size, sizeof(void *)) +
> ALIGN(extra_buffers_size, sizeof(void *)) -
> - ALIGN(secctx_sz, sizeof(u64));
> + ALIGN(lsmctx.len, sizeof(u64));
>
> t->security_ctx = t->buffer->user_data + buf_offset;
> err = binder_alloc_copy_to_buffer(&target_proc->alloc,
> t->buffer, buf_offset,
> - secctx, secctx_sz);
> + lsmctx.context, lsmctx.len);
> if (err) {
> t->security_ctx = 0;
> WARN_ON(1);
> }
> - security_release_secctx(secctx, secctx_sz);
> - secctx = NULL;
> + security_release_secctx(&lsmctx);
> + lsmctx.context = NULL;
> }
> t->buffer->debug_id = t->debug_id;
> t->buffer->transaction = t;
> @@ -3394,7 +3394,7 @@ static void binder_transaction(struct binder_proc *proc,
> off_end_offset = off_start_offset + tr->offsets_size;
> sg_buf_offset = ALIGN(off_end_offset, sizeof(void *));
> sg_buf_end_offset = sg_buf_offset + extra_buffers_size -
> - ALIGN(secctx_sz, sizeof(u64));
> + ALIGN(lsmctx.len, sizeof(u64));
> off_min = 0;
> for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
> buffer_offset += sizeof(binder_size_t)) {
> @@ -3773,8 +3773,8 @@ static void binder_transaction(struct binder_proc *proc,
> binder_alloc_free_buf(&target_proc->alloc, t->buffer);
> err_binder_alloc_buf_failed:
> err_bad_extra_size:
> - if (secctx)
> - security_release_secctx(secctx, secctx_sz);
> + if (lsmctx.context)
> + security_release_secctx(&lsmctx);
> err_get_secctx_failed:
> kfree(tcomplete);
> binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index e066a556eccb..f7996770cc2c 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -1446,12 +1446,16 @@ int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
>
> void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
> {
> +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> + struct lsm_context scaff; /* scaffolding */
> +#endif
> #ifdef CONFIG_CEPH_FS_POSIX_ACL
> posix_acl_release(as_ctx->acl);
> posix_acl_release(as_ctx->default_acl);
> #endif
> #ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> - security_release_secctx(as_ctx->sec_ctx, as_ctx->sec_ctxlen);
> + lsmcontext_init(&scaff, as_ctx->sec_ctx, as_ctx->sec_ctxlen, 0);
> + security_release_secctx(&scaff);
> #endif
> #ifdef CONFIG_FS_ENCRYPTION
> kfree(as_ctx->fscrypt_auth);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index cd2fbde2e6d7..76776d716744 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -138,8 +138,12 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> static inline void
> nfs4_label_release_security(struct nfs4_label *label)
> {
> - if (label)
> - security_release_secctx(label->label, label->len);
> + struct lsm_context scaff; /* scaffolding */
> +
> + if (label) {
> + lsmcontext_init(&scaff, label->label, label->len, 0);
> + security_release_secctx(&scaff);
> + }
> }
> static inline u32 *nfs4_bitmask(struct nfs_server *server, struct nfs4_label *label)
> {
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index f118921250c3..537ad363d70a 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3653,8 +3653,12 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
>
> out:
> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> - if (args.context)
> - security_release_secctx(args.context, args.contextlen);
> + if (args.context) {
> + struct lsm_context scaff; /* scaffolding */
> +
> + lsmcontext_init(&scaff, args.context, args.contextlen, 0);
> + security_release_secctx(&scaff);
> + }
> #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
> kfree(args.acl);
> if (tempfh) {
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index eb2937599cb0..c13df23132eb 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -300,7 +300,7 @@ LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
> LSM_HOOK(int, -EOPNOTSUPP, lsmprop_to_secctx, struct lsm_prop *prop,
> char **secdata, u32 *seclen)
> LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
> -LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)
> +LSM_HOOK(void, LSM_RET_VOID, release_secctx, struct lsm_context *cp)
> LSM_HOOK(void, LSM_RET_VOID, inode_invalidate_secctx, struct inode *inode)
> LSM_HOOK(int, 0, inode_notifysecctx, struct inode *inode, void *ctx, u32 ctxlen)
> LSM_HOOK(int, 0, inode_setsecctx, struct dentry *dentry, void *ctx, u32 ctxlen)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index fd690fa73162..1a3ca02224e8 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -225,6 +225,37 @@ extern unsigned long dac_mmap_min_addr;
> #define dac_mmap_min_addr 0UL
> #endif
>
> +/*
> + * A "security context" is the text representation of
> + * the information used by LSMs.
> + * This structure contains the string, its length, and which LSM
> + * it is useful for.
> + */
> +struct lsm_context {
> + char *context; /* Provided by the module */
> + u32 len;
> + int id; /* Identifies the module */
> +};
> +
> +/**
> + * lsmcontext_init - initialize an lsmcontext structure.
> + * @cp: Pointer to the context to initialize
> + * @context: Initial context, or NULL
> + * @size: Size of context, or 0
> + * @id: Which LSM provided the context
> + *
> + * Fill in the lsmcontext from the provided information.
> + * This is a scaffolding function that will be removed when
> + * lsm_context integration is complete.
> + */
> +static inline void lsmcontext_init(struct lsm_context *cp, char *context,
> + u32 size, int id)
> +{
> + cp->id = id;
> + cp->context = context;
> + cp->len = size;
> +}
> +
> /*
> * Values used in the task_security_ops calls
> */
> @@ -556,7 +587,7 @@ int security_ismaclabel(const char *name);
> int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> int security_lsmprop_to_secctx(struct lsm_prop *prop, char **secdata, u32 *seclen);
> int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
> -void security_release_secctx(char *secdata, u32 seclen);
> +void security_release_secctx(struct lsm_context *cp);
> void security_inode_invalidate_secctx(struct inode *inode);
> int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> @@ -1545,7 +1576,7 @@ static inline int security_secctx_to_secid(const char *secdata,
> return -EOPNOTSUPP;
> }
>
> -static inline void security_release_secctx(char *secdata, u32 seclen)
> +static inline void security_release_secctx(struct lsm_context *cp)
> {
> }
>
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 0d35c7c77a74..f75449e1d876 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -105,16 +105,17 @@ static __inline__ int scm_send(struct socket *sock, struct msghdr *msg,
> #ifdef CONFIG_SECURITY_NETWORK
> static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
> {
> - char *secdata;
> - u32 seclen;
> + struct lsm_context ctx;
> int err;
>
> if (test_bit(SOCK_PASSSEC, &sock->flags)) {
> - err = security_secid_to_secctx(scm->secid, &secdata, &seclen);
> + err = security_secid_to_secctx(scm->secid, &ctx.context,
> + &ctx.len);
>
> if (!err) {
> - put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
> - security_release_secctx(secdata, seclen);
> + put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, ctx.len,
> + ctx.context);
> + security_release_secctx(&ctx);
> }
> }
> }
> diff --git a/kernel/audit.c b/kernel/audit.c
> index d2797e8fe182..bafd8fd2b1f3 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1221,8 +1221,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct audit_buffer *ab;
> u16 msg_type = nlh->nlmsg_type;
> struct audit_sig_info *sig_data;
> - char *ctx = NULL;
> - u32 len;
> + struct lsm_context lsmctx;
>
> err = audit_netlink_ok(skb, msg_type);
> if (err)
> @@ -1472,27 +1471,29 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
> break;
> }
> case AUDIT_SIGNAL_INFO:
> - len = 0;
> if (lsmprop_is_set(&audit_sig_lsm)) {
> - err = security_lsmprop_to_secctx(&audit_sig_lsm, &ctx,
> - &len);
> + err = security_lsmprop_to_secctx(&audit_sig_lsm,
> + &lsmctx.context,
> + &lsmctx.len);
> if (err)
> return err;
> }
> - sig_data = kmalloc(struct_size(sig_data, ctx, len), GFP_KERNEL);
> + sig_data = kmalloc(struct_size(sig_data, ctx, lsmctx.len),
> + GFP_KERNEL);
> if (!sig_data) {
> if (lsmprop_is_set(&audit_sig_lsm))
> - security_release_secctx(ctx, len);
> + security_release_secctx(&lsmctx);
> return -ENOMEM;
> }
> sig_data->uid = from_kuid(&init_user_ns, audit_sig_uid);
> sig_data->pid = audit_sig_pid;
> if (lsmprop_is_set(&audit_sig_lsm)) {
> - memcpy(sig_data->ctx, ctx, len);
> - security_release_secctx(ctx, len);
> + memcpy(sig_data->ctx, lsmctx.context, lsmctx.len);
> + security_release_secctx(&lsmctx);
> }
> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
> - sig_data, struct_size(sig_data, ctx, len));
> + sig_data, struct_size(sig_data, ctx,
> + lsmctx.len));
> kfree(sig_data);
> break;
> case AUDIT_TTY_GET: {
> @@ -2180,23 +2181,22 @@ void audit_log_key(struct audit_buffer *ab, char *key)
> int audit_log_task_context(struct audit_buffer *ab)
> {
> struct lsm_prop prop;
> - char *ctx = NULL;
> - unsigned len;
> + struct lsm_context ctx;
> int error;
>
> security_current_getlsmprop_subj(&prop);
> if (!lsmprop_is_set(&prop))
> return 0;
>
> - error = security_lsmprop_to_secctx(&prop, &ctx, &len);
> + error = security_lsmprop_to_secctx(&prop, &ctx.context, &ctx.len);
> if (error) {
> if (error != -EINVAL)
> goto error_path;
> return 0;
> }
>
> - audit_log_format(ab, " subj=%s", ctx);
> - security_release_secctx(ctx, len);
> + audit_log_format(ab, " subj=%s", ctx.context);
> + security_release_secctx(&ctx);
> return 0;
>
> error_path:
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index f28fd513d047..c196dd4c9b54 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1098,8 +1098,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> char *comm)
> {
> struct audit_buffer *ab;
> - char *ctx = NULL;
> - u32 len;
> + struct lsm_context ctx;
> int rc = 0;
>
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
> @@ -1110,12 +1109,12 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> from_kuid(&init_user_ns, auid),
> from_kuid(&init_user_ns, uid), sessionid);
> if (lsmprop_is_set(prop)) {
> - if (security_lsmprop_to_secctx(prop, &ctx, &len)) {
> + if (security_lsmprop_to_secctx(prop, &ctx.context, &ctx.len)) {
> audit_log_format(ab, " obj=(none)");
> rc = 1;
> } else {
> - audit_log_format(ab, " obj=%s", ctx);
> - security_release_secctx(ctx, len);
> + audit_log_format(ab, " obj=%s", ctx.context);
> + security_release_secctx(&ctx);
> }
> }
> audit_log_format(ab, " ocomm=");
> @@ -1371,6 +1370,7 @@ static void audit_log_time(struct audit_context *context, struct audit_buffer **
>
> static void show_special(struct audit_context *context, int *call_panic)
> {
> + struct lsm_context lsmcxt;
> struct audit_buffer *ab;
> int i;
>
> @@ -1401,7 +1401,8 @@ static void show_special(struct audit_context *context, int *call_panic)
> *call_panic = 1;
> } else {
> audit_log_format(ab, " obj=%s", ctx);
> - security_release_secctx(ctx, len);
> + lsmcontext_init(&lsmcxt, ctx, len, 0);
> + security_release_secctx(&lsmcxt);
> }
> }
> if (context->ipc.has_perm) {
> @@ -1560,15 +1561,15 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
> MAJOR(n->rdev),
> MINOR(n->rdev));
> if (lsmprop_is_set(&n->oprop)) {
> - char *ctx = NULL;
> - u32 len;
> + struct lsm_context ctx;
>
> - if (security_lsmprop_to_secctx(&n->oprop, &ctx, &len)) {
> + if (security_lsmprop_to_secctx(&n->oprop, &ctx.context,
> + &ctx.len)) {
> if (call_panic)
> *call_panic = 2;
> } else {
> - audit_log_format(ab, " obj=%s", ctx);
> - security_release_secctx(ctx, len);
> + audit_log_format(ab, " obj=%s", ctx.context);
> + security_release_secctx(&ctx);
> }
> }
>
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index cf377377b52d..a8180dcc2a32 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -128,20 +128,20 @@ static void ip_cmsg_recv_checksum(struct msghdr *msg, struct sk_buff *skb,
>
> static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
> {
> - char *secdata;
> - u32 seclen, secid;
> + struct lsm_context ctx;
> + u32 secid;
> int err;
>
> err = security_socket_getpeersec_dgram(NULL, skb, &secid);
> if (err)
> return;
>
> - err = security_secid_to_secctx(secid, &secdata, &seclen);
> + err = security_secid_to_secctx(secid, &ctx.context, &ctx.len);
> if (err)
> return;
>
> - put_cmsg(msg, SOL_IP, SCM_SECURITY, seclen, secdata);
> - security_release_secctx(secdata, seclen);
> + put_cmsg(msg, SOL_IP, SCM_SECURITY, ctx.len, ctx.context);
> + security_release_secctx(&ctx);
> }
>
> static void ip_cmsg_recv_dstaddr(struct msghdr *msg, struct sk_buff *skb)
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 6a1239433830..86a57a3afdd6 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -357,10 +357,10 @@ static int ctnetlink_dump_mark(struct sk_buff *skb, const struct nf_conn *ct,
> static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
> {
> struct nlattr *nest_secctx;
> - int len, ret;
> - char *secctx;
> + struct lsm_context ctx;
> + int ret;
>
> - ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
> + ret = security_secid_to_secctx(ct->secmark, &ctx.context, &ctx.len);
> if (ret)
> return 0;
>
> @@ -369,13 +369,13 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
> if (!nest_secctx)
> goto nla_put_failure;
>
> - if (nla_put_string(skb, CTA_SECCTX_NAME, secctx))
> + if (nla_put_string(skb, CTA_SECCTX_NAME, ctx.context))
> goto nla_put_failure;
> nla_nest_end(skb, nest_secctx);
>
> ret = 0;
> nla_put_failure:
> - security_release_secctx(secctx, len);
> + security_release_secctx(&ctx);
> return ret;
> }
> #else
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 7d4f0fa8b609..5f7fd23b7afe 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -172,17 +172,16 @@ static void ct_seq_stop(struct seq_file *s, void *v)
> #ifdef CONFIG_NF_CONNTRACK_SECMARK
> static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> {
> + struct lsm_context ctx;
> int ret;
> - u32 len;
> - char *secctx;
>
> - ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
> + ret = security_secid_to_secctx(ct->secmark, &ctx.context, &ctx.len);
> if (ret)
> return;
>
> - seq_printf(s, "secctx=%s ", secctx);
> + seq_printf(s, "secctx=%s ", ctx.context);
>
> - security_release_secctx(secctx, len);
> + security_release_secctx(&ctx);
> }
> #else
> static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index d2773ce9b585..37757cd77cf1 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -567,6 +567,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> enum ip_conntrack_info ctinfo = 0;
> const struct nfnl_ct_hook *nfnl_ct;
> bool csum_verify;
> + struct lsm_context scaff; /* scaffolding */
> char *secdata = NULL;
> u32 seclen = 0;
> ktime_t tstamp;
> @@ -810,8 +811,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> }
>
> nlh->nlmsg_len = skb->len;
> - if (seclen)
> - security_release_secctx(secdata, seclen);
> + if (seclen) {
> + lsmcontext_init(&scaff, secdata, seclen, 0);
> + security_release_secctx(&scaff);
> + }
> return skb;
>
> nla_put_failure:
> @@ -819,8 +822,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> kfree_skb(skb);
> net_err_ratelimited("nf_queue: error creating packet message\n");
> nlmsg_failure:
> - if (seclen)
> - security_release_secctx(secdata, seclen);
> + if (seclen) {
> + lsmcontext_init(&scaff, secdata, seclen, 0);
> + security_release_secctx(&scaff);
> + }
> return NULL;
> }
>
> 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
> @@ -374,8 +374,7 @@ int netlbl_unlhsh_add(struct net *net,
> struct net_device *dev;
> struct netlbl_unlhsh_iface *iface;
> struct audit_buffer *audit_buf = NULL;
> - char *secctx = NULL;
> - u32 secctx_len;
> + struct lsm_context ctx;
>
> if (addr_len != sizeof(struct in_addr) &&
> addr_len != sizeof(struct in6_addr))
> @@ -438,11 +437,10 @@ int netlbl_unlhsh_add(struct net *net,
> unlhsh_add_return:
> rcu_read_unlock();
> if (audit_buf != NULL) {
> - if (security_secid_to_secctx(secid,
> - &secctx,
> - &secctx_len) == 0) {
> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + if (security_secid_to_secctx(secid, &ctx.context,
> + &ctx.len) == 0) {
> + audit_log_format(audit_buf, " sec_obj=%s", ctx.context);
> + security_release_secctx(&ctx);
> }
> audit_log_format(audit_buf, " res=%u", ret_val == 0 ? 1 : 0);
> audit_log_end(audit_buf);
> @@ -473,8 +471,7 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> struct netlbl_unlhsh_addr4 *entry;
> struct audit_buffer *audit_buf;
> struct net_device *dev;
> - char *secctx;
> - u32 secctx_len;
> + struct lsm_context ctx;
>
> spin_lock(&netlbl_unlhsh_lock);
> list_entry = netlbl_af4list_remove(addr->s_addr, mask->s_addr,
> @@ -494,10 +491,10 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> addr->s_addr, mask->s_addr);
> dev_put(dev);
> if (entry != NULL &&
> - security_secid_to_secctx(entry->secid,
> - &secctx, &secctx_len) == 0) {
> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + security_secid_to_secctx(entry->secid, &ctx.context,
> + &ctx.len) == 0) {
> + audit_log_format(audit_buf, " sec_obj=%s", ctx.context);
> + security_release_secctx(&ctx);
> }
> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
> audit_log_end(audit_buf);
> @@ -534,8 +531,7 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> struct netlbl_unlhsh_addr6 *entry;
> struct audit_buffer *audit_buf;
> struct net_device *dev;
> - char *secctx;
> - u32 secctx_len;
> + struct lsm_context ctx;
>
> spin_lock(&netlbl_unlhsh_lock);
> list_entry = netlbl_af6list_remove(addr, mask, &iface->addr6_list);
> @@ -554,10 +550,10 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> addr, mask);
> dev_put(dev);
> if (entry != NULL &&
> - security_secid_to_secctx(entry->secid,
> - &secctx, &secctx_len) == 0) {
> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + security_secid_to_secctx(entry->secid, &ctx.context,
> + &ctx.len) == 0) {
> + audit_log_format(audit_buf, " sec_obj=%s", ctx.context);
> + security_release_secctx(&ctx);
> }
> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
> audit_log_end(audit_buf);
> @@ -1069,10 +1065,9 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> int ret_val = -ENOMEM;
> struct netlbl_unlhsh_walk_arg *cb_arg = arg;
> struct net_device *dev;
> + struct lsm_context ctx;
> void *data;
> u32 secid;
> - char *secctx;
> - u32 secctx_len;
>
> data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
> cb_arg->seq, &netlbl_unlabel_gnl_family,
> @@ -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);
> + security_release_secctx(&ctx);
> if (ret_val != 0)
> goto list_cb_failure;
>
> diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
> index 81635a13987b..f5e7a9919df1 100644
> --- a/net/netlabel/netlabel_user.c
> +++ b/net/netlabel/netlabel_user.c
> @@ -84,8 +84,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
> struct netlbl_audit *audit_info)
> {
> struct audit_buffer *audit_buf;
> - char *secctx;
> - u32 secctx_len;
> + struct lsm_context ctx;
>
> if (audit_enabled == AUDIT_OFF)
> return NULL;
> @@ -99,10 +98,10 @@ struct audit_buffer *netlbl_audit_start_common(int type,
> audit_info->sessionid);
>
> if (lsmprop_is_set(&audit_info->prop) &&
> - security_lsmprop_to_secctx(&audit_info->prop, &secctx,
> - &secctx_len) == 0) {
> - audit_log_format(audit_buf, " subj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + security_lsmprop_to_secctx(&audit_info->prop, &ctx.context,
> + &ctx.len) == 0) {
> + audit_log_format(audit_buf, " subj=%s", ctx.context);
> + security_release_secctx(&ctx);
> }
>
> return audit_buf;
> diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h
> index cc6d1c9f4a47..8b92f90b6921 100644
> --- a/security/apparmor/include/secid.h
> +++ b/security/apparmor/include/secid.h
> @@ -29,7 +29,7 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> int apparmor_lsmprop_to_secctx(struct lsm_prop *prop, char **secdata,
> u32 *seclen);
> int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
> -void apparmor_release_secctx(char *secdata, u32 seclen);
> +void apparmor_release_secctx(struct lsm_context *cp);
>
>
> int aa_alloc_secid(struct aa_label *label, gfp_t gfp);
> diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
> index 6350d107013a..8d9ced8cdffd 100644
> --- a/security/apparmor/secid.c
> +++ b/security/apparmor/secid.c
> @@ -120,9 +120,16 @@ int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> return 0;
> }
>
> -void apparmor_release_secctx(char *secdata, u32 seclen)
> +void apparmor_release_secctx(struct lsm_context *cp)
> {
> - kfree(secdata);
> + /*
> + * stacking scaffolding:
> + * When it is possible for more than one LSM to provide a
> + * release hook, do this check:
> + * if (cp->id == LSM_ID_APPARMOR || cp->id == LSM_ID_UNDEF)
> + */
> +
> + kfree(cp->context);
> }
>
> /**
> diff --git a/security/security.c b/security/security.c
> index 0003d5ace5cc..0c9c3a02704b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4365,14 +4365,14 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>
> /**
> * security_release_secctx() - Free a secctx buffer
> - * @secdata: secctx
> - * @seclen: length of secctx
> + * @cp: the security context
> *
> * Release the security context.
> */
> -void security_release_secctx(char *secdata, u32 seclen)
> +void security_release_secctx(struct lsm_context *cp)
> {
> - call_void_hook(release_secctx, secdata, seclen);
> + call_void_hook(release_secctx, cp);
> + memset(cp, 0, sizeof(*cp));
> }
> EXPORT_SYMBOL(security_release_secctx);
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 025b60c5b605..1503d398c87d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6624,9 +6624,16 @@ static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> secid, GFP_KERNEL);
> }
>
> -static void selinux_release_secctx(char *secdata, u32 seclen)
> +static void selinux_release_secctx(struct lsm_context *cp)
> {
> - kfree(secdata);
> + /*
> + * stacking scaffolding:
> + * When it is possible for more than one LSM to provide a
> + * release hook, do this check:
> + * if (cp->id == LSM_ID_SELINUX || cp->id == LSM_ID_UNDEF)
> + */
> +
> + kfree(cp->context);
> }
>
> static void selinux_inode_invalidate_secctx(struct inode *inode)
> --
> 2.46.0
>
Powered by blists - more mailing lists