[PATCH v39 17/42] LSM: Use lsmcontext in security_secid_to_secctx

From: Casey Schaufler
Date: Fri Dec 15 2023 - 17:30:00 EST


Replace the (secctx,seclen) pointer pair with a single
lsmcontext 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.

security_secid_to_secctx() will now return the length value
on success instead of 0.

Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
Cc: netdev@xxxxxxxxxxxxxxx
Cc: audit@xxxxxxxxxxxxxxx
Cc: netfilter-devel@xxxxxxxxxxxxxxx
Cc: Todd Kjos <tkjos@xxxxxxxxxx>
---
drivers/android/binder.c | 5 ++---
include/linux/lsm_hook_defs.h | 3 +--
include/linux/security.h | 5 ++---
include/net/scm.h | 5 ++---
net/ipv4/ip_sockglue.c | 4 ++--
net/netfilter/nf_conntrack_netlink.c | 8 ++++----
net/netfilter/nf_conntrack_standalone.c | 4 ++--
net/netfilter/nfnetlink_queue.c | 27 ++++++++++---------------
net/netlabel/netlabel_unlabeled.c | 13 +++++-------
security/apparmor/include/secid.h | 2 +-
security/apparmor/secid.c | 13 +++++++-----
security/security.c | 17 ++++++++--------
security/selinux/hooks.c | 17 ++++++++++++++--
security/smack/smack_lsm.c | 16 ++++++++-------
14 files changed, 72 insertions(+), 67 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 58bdb5b75131..c0fa95e64e7c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3199,9 +3199,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, &lsmctx.context,
- &lsmctx.len);
- if (ret) {
+ ret = security_secid_to_secctx(secid, &lsmctx);
+ if (ret < 0) {
binder_txn_error("%d:%d failed to get security context\n",
thread->pid, proc->pid);
return_error = BR_FAILED_REPLY;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index c5e5a32f5e07..8e0155ac6697 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -273,8 +273,7 @@ LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name,
char **value)
LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
LSM_HOOK(int, 0, ismaclabel, const char *name)
-LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
- u32 *seclen)
+LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, struct lsmcontext *cp)
LSM_HOOK(int, -EOPNOTSUPP, lsmblob_to_secctx, struct lsmblob *blob,
char **secdata, u32 *seclen)
LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
diff --git a/include/linux/security.h b/include/linux/security.h
index 9712056d71a0..03b79089eaf7 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -562,7 +562,7 @@ int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_ismaclabel(const char *name);
-int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
+int security_secid_to_secctx(u32 secid, struct lsmcontext *cp);
int security_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
u32 *seclen);
int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
@@ -1487,8 +1487,7 @@ static inline int security_ismaclabel(const char *name)
return 0;
}

-static inline int security_secid_to_secctx(u32 secid, char **secdata,
- u32 *seclen)
+static inline int security_secid_to_secctx(u32 secid, struct lsmcontext *cp)
{
return -EOPNOTSUPP;
}
diff --git a/include/net/scm.h b/include/net/scm.h
index 6e1add51d4c2..91452b36b5bf 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -97,10 +97,9 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
int err;

if (test_bit(SOCK_PASSSEC, &sock->flags)) {
- err = security_secid_to_secctx(scm->secid, &ctx.context,
- &ctx.len);
+ err = security_secid_to_secctx(scm->secid, &ctx);

- if (!err) {
+ if (err >= 0) {
put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, ctx.len,
ctx.context);
security_release_secctx(&ctx);
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 3bf8ff9d4434..38b9f822a70d 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -138,8 +138,8 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
if (err)
return;

- err = security_secid_to_secctx(secid, &ctx.context, &ctx.len);
- if (err)
+ err = security_secid_to_secctx(secid, &ctx);
+ if (err < 0)
return;

put_cmsg(msg, SOL_IP, SCM_SECURITY, ctx.len, ctx.context);
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 3e79b339a1bc..a7dfc39bfbf3 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -360,8 +360,8 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
struct lsmcontext ctx;
int ret;

- ret = security_secid_to_secctx(ct->secmark, &ctx.context, &ctx.len);
- if (ret)
+ ret = security_secid_to_secctx(ct->secmark, &ctx);
+ if (ret < 0)
return 0;

ret = -1;
@@ -669,8 +669,8 @@ static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
#ifdef CONFIG_NF_CONNTRACK_SECMARK
int len, ret;

- ret = security_secid_to_secctx(ct->secmark, NULL, &len);
- if (ret)
+ ret = security_secid_to_secctx(ct->secmark, NULL);
+ if (ret < 0)
return 0;

return nla_total_size(0) /* CTA_SECCTX */
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 23949d233375..a1d8952db1c1 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -178,8 +178,8 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
struct lsmcontext ctx;
int ret;

- ret = security_secid_to_secctx(ct->secmark, &ctx.context, &ctx.len);
- if (ret)
+ ret = security_secid_to_secctx(ct->secmark, &ctx);
+ if (ret < 0)
return;

seq_printf(s, "secctx=%s ", ctx.context);
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 8b4c5c08daa7..f7918b21672d 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -319,18 +319,18 @@ static int nfqnl_put_sk_classid(struct sk_buff *skb, struct sock *sk)
return 0;
}

-static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
+static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, struct lsmcontext *ctx)
{
u32 seclen = 0;
#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
+
if (!skb || !sk_fullsock(skb->sk))
return 0;

read_lock_bh(&skb->sk->sk_callback_lock);

if (skb->secmark)
- security_secid_to_secctx(skb->secmark, secdata, &seclen);
-
+ seclen = security_secid_to_secctx(skb->secmark, ctx);
read_unlock_bh(&skb->sk->sk_callback_lock);
#endif
return seclen;
@@ -408,8 +408,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 lsmcontext scaff; /* scaffolding */
- char *secdata = NULL;
+ struct lsmcontext ctx;
u32 seclen = 0;
ktime_t tstamp;

@@ -484,8 +483,8 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
}

if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
- seclen = nfqnl_get_sk_secctx(entskb, &secdata);
- if (seclen)
+ seclen = nfqnl_get_sk_secctx(entskb, &ctx);
+ if (seclen >= 0)
size += nla_total_size(seclen);
}

@@ -624,7 +623,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
if (nfqnl_put_sk_classid(skb, entskb->sk) < 0)
goto nla_put_failure;

- if (seclen && nla_put(skb, NFQA_SECCTX, seclen, secdata))
+ if (seclen && nla_put(skb, NFQA_SECCTX, ctx.len, ctx.context))
goto nla_put_failure;

if (ct && nfnl_ct->build(skb, ct, ctinfo, NFQA_CT, NFQA_CT_INFO) < 0)
@@ -652,10 +651,8 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
}

nlh->nlmsg_len = skb->len;
- if (seclen) {
- lsmcontext_init(&scaff, secdata, seclen, 0);
- security_release_secctx(&scaff);
- }
+ if (seclen >= 0)
+ security_release_secctx(&ctx);
return skb;

nla_put_failure:
@@ -663,10 +660,8 @@ 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) {
- lsmcontext_init(&scaff, secdata, seclen, 0);
- security_release_secctx(&scaff);
- }
+ if (seclen >= 0)
+ security_release_secctx(&ctx);
return NULL;
}

diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 464105080245..b43cfb4fe4f1 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -437,8 +437,7 @@ int netlbl_unlhsh_add(struct net *net,
unlhsh_add_return:
rcu_read_unlock();
if (audit_buf != NULL) {
- if (security_secid_to_secctx(secid, &ctx.context,
- &ctx.len) == 0) {
+ if (security_secid_to_secctx(secid, &ctx) >= 0) {
audit_log_format(audit_buf, " sec_obj=%s", ctx.context);
security_release_secctx(&ctx);
}
@@ -491,8 +490,7 @@ 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, &ctx.context,
- &ctx.len) == 0) {
+ security_secid_to_secctx(entry->secid, &ctx) >= 0) {
audit_log_format(audit_buf, " sec_obj=%s", ctx.context);
security_release_secctx(&ctx);
}
@@ -550,8 +548,7 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
addr, mask);
dev_put(dev);
if (entry != NULL &&
- security_secid_to_secctx(entry->secid, &ctx.context,
- &ctx.len) == 0) {
+ security_secid_to_secctx(entry->secid, &ctx) >= 0) {
audit_log_format(audit_buf, " sec_obj=%s", ctx.context);
security_release_secctx(&ctx);
}
@@ -1122,8 +1119,8 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
secid = addr6->secid;
}

- ret_val = security_secid_to_secctx(secid, &ctx.context, &ctx.len);
- if (ret_val != 0)
+ ret_val = security_secid_to_secctx(secid, &ctx);
+ if (ret_val < 0)
goto list_cb_failure;
ret_val = nla_put(cb_arg->skb,
NLBL_UNLABEL_A_SECCTX,
diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h
index e47c37c1beda..b66c2d043a02 100644
--- a/security/apparmor/include/secid.h
+++ b/security/apparmor/include/secid.h
@@ -25,7 +25,7 @@ struct aa_label;
extern int apparmor_display_secid_mode;

struct aa_label *aa_secid_to_label(u32 secid);
-int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
+int apparmor_secid_to_secctx(u32 secid, struct lsmcontext *cp);
int apparmor_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
u32 *seclen);
int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
index e9f655f54a42..55d6c54fe90e 100644
--- a/security/apparmor/secid.c
+++ b/security/apparmor/secid.c
@@ -61,7 +61,7 @@ struct aa_label *aa_secid_to_label(u32 secid)
return xa_load(&aa_secids, secid);
}

-int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+int apparmor_secid_to_secctx(u32 secid, struct lsmcontext *cp)
{
/* TODO: cache secctx and ref count so we don't have to recreate */
struct aa_label *label = aa_secid_to_label(secid);
@@ -76,8 +76,8 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
if (apparmor_display_secid_mode)
flags |= FLAG_SHOW_MODE;

- if (secdata)
- len = aa_label_asxprint(secdata, root_ns, label,
+ if (cp)
+ len = aa_label_asxprint(&cp->context, root_ns, label,
flags, GFP_ATOMIC);
else
len = aa_label_snxprint(NULL, 0, root_ns, label, flags);
@@ -85,9 +85,12 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
if (len < 0)
return -ENOMEM;

- *seclen = len;
+ if (cp) {
+ cp->len = len;
+ cp->id = LSM_ID_APPARMOR;
+ }

- return 0;
+ return len;
}

int apparmor_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
diff --git a/security/security.c b/security/security.c
index 063a209ac17f..708a26a88447 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4172,17 +4172,16 @@ EXPORT_SYMBOL(security_ismaclabel);
/**
* security_secid_to_secctx() - Convert a secid to a secctx
* @secid: secid
- * @secdata: secctx
- * @seclen: secctx length
+ * @cp: the LSM context
*
- * Convert secid to security context. If @secdata is NULL the length of the
- * result will be returned in @seclen, but no @secdata will be returned. This
+ * Convert secid to security context. If @cp is NULL the length of the
+ * result will be returned, but no data will be returned. This
* does mean that the length could change between calls to check the length and
- * the next call which actually allocates and returns the @secdata.
+ * the next call which actually allocates and returns the data.
*
- * Return: Return 0 on success, error on failure.
+ * Return: Return length of data on success, error on failure.
*/
-int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+int security_secid_to_secctx(u32 secid, struct lsmcontext *cp)
{
struct security_hook_list *hp;
int rc;
@@ -4192,7 +4191,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
* LSM hook is not "stackable").
*/
hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
- rc = hp->hook.secid_to_secctx(secid, secdata, seclen);
+ rc = hp->hook.secid_to_secctx(secid, cp);
if (rc != LSM_RET_DEFAULT(secid_to_secctx))
return rc;
}
@@ -4221,7 +4220,7 @@ int security_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
struct security_hook_list *hp;
int rc;

- hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
+ hlist_for_each_entry(hp, &security_hook_heads.lsmblob_to_secctx, list) {
rc = hp->hook.lsmblob_to_secctx(blob, secdata, seclen);
if (rc != LSM_RET_DEFAULT(secid_to_secctx))
return rc;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1a428a6964a0..37b97cf81da1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6565,9 +6565,22 @@ static int selinux_ismaclabel(const char *name)
return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
}

-static int selinux_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+static int selinux_secid_to_secctx(u32 secid, struct lsmcontext *cp)
{
- return security_sid_to_context(secid, secdata, seclen);
+ u32 seclen;
+ u32 ret;
+
+ if (cp) {
+ cp->id = LSM_ID_SELINUX;
+ ret = security_sid_to_context(secid, &cp->context, &cp->len);
+ if (ret < 0)
+ return ret;
+ return cp->len;
+ }
+ ret = security_sid_to_context(secid, NULL, &seclen);
+ if (ret < 0)
+ return ret;
+ return seclen;
}

static int selinux_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 7dab00bbd0ed..d82753bc52ab 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4814,19 +4814,21 @@ static int smack_ismaclabel(const char *name)
/**
* smack_secid_to_secctx - return the smack label for a secid
* @secid: incoming integer
- * @secdata: destination
- * @seclen: how long it is
+ * @cp: destination
*
* Exists for networking code.
*/
-static int smack_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+static int smack_secid_to_secctx(u32 secid, struct lsmcontext *cp)
{
struct smack_known *skp = smack_from_secid(secid);
+ int len = strlen(skp->smk_known);

- if (secdata)
- *secdata = skp->smk_known;
- *seclen = strlen(skp->smk_known);
- return 0;
+ if (cp) {
+ cp->context = skp->smk_known;
+ cp->len = len;
+ cp->id = LSM_ID_SMACK;
+ }
+ return len;
}

/**
--
2.41.0