[PATCH 0/7] Change skb secmarks to x-array indexes

[PATCH 0/7] Change skb secmarks to x-array indexes

[Casey Schaufler]

When security secmarks were added to the Linux network stack there was
only one Linux Security Module (LSM), SELinux.  SELinux already used the
concept of a security ID (secid) as the representation of the security
information about a system subject (active entity) or object (passive
entity). Adding a container for a secid, the secmark, to the sk_buff
structure allowed for efficient transmission of the SELinux secid for
socket based access controls.

Subsequent LSMs have chosen to represent security information more
directly. Smack and AppArmor use pointers to structures containing
relevant information. Alas, these pointers do not fit in the u32 secmark
on most modern architectures. These LSMs are required to provide a secid
mapping to use secmarks.

Even with all LSMs that use secmarks having a secid to reference the
security information the mechanism is imperfect. A system that wants
to use multiple LSMs that use secmarks is constrained by the size
of the secmark. There is no rational way to fit multiple secids in a
secmark. While it would be possible to allow one LSM to use the secmark
and any others to be told it is unavailable, this has been deemed an
unacceptable limitation.

There is a lsm_prop structure available that contains security information
for any LSM that maintains it. The secmark cannot, unfortunately,
contain one. Instead, an x-array of lsm_prop structures is maintained,
and the index (secxa) is used in the secmark instead of the single LSM
restricted secid.

Uses of security_secctx_to_secid() have been changed to
security_secctx_to_lsmprop() in the netfilter and iptables code.
The security_secmark_relabel_packet() function has been updated to accept
an lsm_prop pointer rather than a secid.

To support multiple LSMs using a secmark it is necessary to re-evaluate
which lsm_prop structure represents the current security information
at each step where the secmark can be set. Smack sets the secmark for
every packet.  Netfilter, used by SELinux, Smack and AppArmor, will set
the secmark on selected packets at a later time. If Smack and AppArmor
are active on a system Smack will set the secmark initially, and AppArmor
may reset it by netfilter rule.

https://github.com/cschaufler/lsm-stacking#secmark-xa-7.2-rc5-v1

Casey Schaufler (7):
  net, smack: Create a function to set secmarks
  LSM: Implement x array functions for secmarks
  LSM: Two hooks for manipulating struct lsm_prop
  SELinux: hooks for secctx_to_lsmprop and update_lsmprop
  Smack: hooks for secctx_to_lsmprop and update_lsmprop
  Apparmor: hooks for secctx_to_lsmprop and update_lsmprop
  net, lsm: Change skb secmarks to x-array indexes

 include/linux/lsm_hook_defs.h     |   6 +-
 include/linux/lsm_secxa.h         |  22 ++++++
 include/linux/security.h          |  21 +++++-
 net/netfilter/nfnetlink_queue.c   |  12 ++-
 net/netfilter/nft_meta.c          |  16 ++--
 net/netfilter/xt_CONNSECMARK.c    |   3 +-
 net/netfilter/xt_SECMARK.c        |  15 ++--
 security/Makefile                 |   1 +
 security/apparmor/include/secid.h |   4 +
 security/apparmor/lsm.c           |   2 +
 security/apparmor/net.c           |   8 +-
 security/apparmor/secid.c         |  23 ++++++
 security/lsm_secxa.c              | 119 ++++++++++++++++++++++++++++++
 security/security.c               |  38 +++++++++-
 security/selinux/hooks.c          |  89 +++++++++++++++++++---
 security/smack/smack_lsm.c        |  46 +++++++++++-
 security/smack/smack_netfilter.c  |   9 ++-
 17 files changed, 398 insertions(+), 36 deletions(-)
 create mode 100644 include/linux/lsm_secxa.h
 create mode 100644 security/lsm_secxa.c

-- 
2.54.0

[PATCH 1/7] net, smack: Create a function to set secmarks

[Casey Schaufler]

Rather than open coding assignments to skb->secmark, use a helper function
secxa_set_secmark(). This allows for a case where assigning a secmark
is more complex than a simple assignment. The version of the function
here does the legacy simple assignment.

Change the functions that currently assign values to skb->secmark to
use this function.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_secxa.h        | 22 ++++++++++++++++++++++
 net/netfilter/nft_meta.c         |  5 +++--
 net/netfilter/xt_CONNSECMARK.c   |  3 ++-
 net/netfilter/xt_SECMARK.c       |  3 ++-
 security/smack/smack_netfilter.c |  3 ++-
 5 files changed, 31 insertions(+), 5 deletions(-)
 create mode 100644 include/linux/lsm_secxa.h

diff --git a/include/linux/lsm_secxa.h b/include/linux/lsm_secxa.h
new file mode 100644
index 000000000000..f4c732d26680
--- /dev/null
+++ b/include/linux/lsm_secxa.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2026 Casey Schaufler <casey@schaufler-ca.com>
+ */
+
+#ifndef __LINUX_LSM_SECXA_H
+#define __LINUX_LSM_SECXA_H
+
+#ifdef CONFIG_SECURITY
+
+#include <linux/security.h>
+#include <linux/skbuff.h>
+
+static inline void secxa_set_secmark(struct sk_buff *skb, u32 secxa)
+{
+	skb->secmark = secxa;
+}
+
+#endif /* CONFIG_SECURITY */
+
+#endif  /* __LINUX_LSM_SECXA_H */
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 0a43e0787a68..bd0f7a0931f4 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -17,6 +17,7 @@
 #include <linux/random.h>
 #include <linux/smp.h>
 #include <linux/static_key.h>
+#include <linux/lsm_secxa.h>
 #include <net/dst.h>
 #include <net/ip.h>
 #include <net/sock.h>
@@ -502,7 +503,7 @@ void nft_meta_set_eval(const struct nft_expr *expr,
 		break;
 #ifdef CONFIG_NETWORK_SECMARK
 	case NFT_META_SECMARK:
-		skb->secmark = value;
+		secxa_set_secmark(skb, value);
 		break;
 #endif
 	default:
@@ -950,7 +951,7 @@ static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs,
 	const struct nft_secmark *priv = nft_obj_data(obj);
 	struct sk_buff *skb = pkt->skb;
 
-	skb->secmark = priv->secid;
+	secxa_set_secmark(skb, priv->secid);
 }
 
 static int nft_secmark_obj_init(const struct nft_ctx *ctx,
diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index 1494b3ee30e1..9d799d2459dc 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -14,6 +14,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/module.h>
 #include <linux/skbuff.h>
+#include <linux/lsm_secxa.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_CONNSECMARK.h>
 #include <net/netfilter/nf_conntrack.h>
@@ -55,7 +56,7 @@ static void secmark_restore(struct sk_buff *skb)
 
 		ct = nf_ct_get(skb, &ctinfo);
 		if (ct && ct->secmark)
-			skb->secmark = ct->secmark;
+			secxa_set_secmark(skb, ct->secmark);
 	}
 }
 
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index 5bc5ea505eb9..ea67aa92ddc2 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -11,6 +11,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/module.h>
 #include <linux/security.h>
+#include <linux/lsm_secxa.h>
 #include <linux/skbuff.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_SECMARK.h>
@@ -36,7 +37,7 @@ secmark_tg(struct sk_buff *skb, const struct xt_secmark_target_info_v1 *info)
 		BUG();
 	}
 
-	skb->secmark = secmark;
+	secxa_set_secmark(skb, secmark);
 	return XT_CONTINUE;
 }
 
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index 17ba578b1308..b363c42f252e 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -14,6 +14,7 @@
 #include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv6.h>
 #include <linux/netdevice.h>
+#include <linux/lsm_secxa.h>
 #include <net/inet_sock.h>
 #include <net/net_namespace.h>
 #include "smack.h"
@@ -29,7 +30,7 @@ static unsigned int smack_ip_output(void *priv,
 	if (sk) {
 		ssp = smack_sock(sk);
 		skp = ssp->smk_out;
-		skb->secmark = skp->smk_secid;
+		secxa_set_secmark(skb, skp->smk_secid);
 	}
 
 	return NF_ACCEPT;
-- 
2.54.0

Re: [PATCH 1/7] net, smack: Create a function to set secmarks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The helper `secxa_set_secmark` causes build failures under two valid kernel configurations: when `CONFIG_SECURITY=y` with `CONFIG_NETWORK_SECMARK=n` (invalid struct member access), and when `CONFIG_SECURITY=n` with `CONFIG_NETWORK_SECMARK=y` (implicit function declaration).
--

--- Patch [1]: [PATCH 1/7] net, smack: Create a function to set secmarks ---
commit e74441b7e906ce8624528752b177a2d903d76f44
Author: Casey Schaufler <casey@schaufler-ca.com>

net, smack: Create a function to set secmarks

This patch creates a helper function secxa_set_secmark() to assign a secmark
to an skb, replacing direct assignments to skb->secmark. This allows for
cases where assigning a secmark is more complex.

> diff --git a/include/linux/lsm_secxa.h b/include/linux/lsm_secxa.h
> new file mode 100644
> index 0000000000000..f4c732d26680c
> --- /dev/null
> +++ b/include/linux/lsm_secxa.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2026 Casey Schaufler <casey@schaufler-ca.com>
> + */
> +
> +#ifndef __LINUX_LSM_SECXA_H
> +#define __LINUX_LSM_SECXA_H
> +
> +#ifdef CONFIG_SECURITY
> +
> +#include <linux/security.h>
> +#include <linux/skbuff.h>
> +
> +static inline void secxa_set_secmark(struct sk_buff *skb, u32 secxa)
> +{
> +	skb->secmark = secxa;

[Severity: Low]
Could this cause build failures under certain configurations?

If CONFIG_SECURITY is enabled but CONFIG_NETWORK_SECMARK is disabled,
skb->secmark won't exist in struct sk_buff, which would result in an invalid
member access error here.

Additionally, if CONFIG_SECURITY is disabled but CONFIG_NETWORK_SECMARK is
enabled, this function won't be declared. This would cause implicit function
declaration errors in modules like xt_CONNSECMARK.c that call it.

Should this helper be guarded by CONFIG_NETWORK_SECMARK instead of
CONFIG_SECURITY, or perhaps both?

> +}
> +
> +#endif /* CONFIG_SECURITY */
> +
> +#endif  /* __LINUX_LSM_SECXA_H */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=1

[PATCH 2/7] LSM: Implement x array functions for secmarks

[Casey Schaufler]

Implement, but don't use (yet) the functions required to use
xarray indexes in secmarks.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_secxa.h |  10 ++--
 security/Makefile         |   1 +
 security/lsm_secxa.c      | 107 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 5 deletions(-)
 create mode 100644 security/lsm_secxa.c

diff --git a/include/linux/lsm_secxa.h b/include/linux/lsm_secxa.h
index f4c732d26680..ffdc354b93fc 100644
--- a/include/linux/lsm_secxa.h
+++ b/include/linux/lsm_secxa.h
@@ -10,12 +10,12 @@
 #ifdef CONFIG_SECURITY
 
 #include <linux/security.h>
-#include <linux/skbuff.h>
 
-static inline void secxa_set_secmark(struct sk_buff *skb, u32 secxa)
-{
-	skb->secmark = secxa;
-}
+struct sk_buff;
+
+int secxa_from_lsmprop(struct lsm_prop *prop);
+int secxa_get_lsmprop(struct lsm_prop **pro, u32 secxa);
+void secxa_set_secmark(struct sk_buff *skb, u32 secxa);
 
 #endif /* CONFIG_SECURITY */
 
diff --git a/security/Makefile b/security/Makefile
index 4601230ba442..e93be00bb6ae 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_KEYS)			+= keys/
 # always enable default capabilities
 obj-y					+= commoncap.o
 obj-$(CONFIG_SECURITY) 			+= lsm_syscalls.o
+obj-$(CONFIG_NETWORK_SECMARK)		+= lsm_secxa.o
 obj-$(CONFIG_MMU)			+= min_addr.o
 
 # Object file lists
diff --git a/security/lsm_secxa.c b/security/lsm_secxa.c
new file mode 100644
index 000000000000..5b67d8218fd2
--- /dev/null
+++ b/security/lsm_secxa.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * Implement functions supporting an x array for LSM properties.
+ *
+ * Copyright (C) 2026 Casey Schaufler <casey@schaufler-ca.com>
+ */
+#define pr_fmt(fmt) "secxa: "fmt
+
+#include <linux/xarray.h>
+#include <linux/export.h>
+#include <linux/security.h>
+#include <linux/lsm_secxa.h>
+#include <linux/skbuff.h>
+
+/*
+ * An Xarray of lsm_prop structures.
+ */
+struct xarray secxa_xa;
+
+/**
+ * secxa_init - initialize the xarry of lsm_prop structures.
+ */
+static int __init secxa_init(void)
+{
+	xa_init_flags(&secxa_xa, XA_FLAGS_ALLOC1);
+
+	return 0;
+}
+core_initcall(secxa_init);
+
+/**
+ * secxa_get_lsmprop - get the lsm_prop associated with a secxa
+ * @pro: destination for the lsm_prop pointer
+ * @secxa: index to look up
+ *
+ * Find the lsm_prop associated with @secxa and place a pointer
+ * to it in @pro.
+ *
+ * Returns 0, or -EINVAL if the mapping can't be found.
+ */
+int secxa_get_lsmprop(struct lsm_prop **pro, u32 secxa)
+{
+	struct lsm_prop *lp;
+
+	if (!secxa)
+		return -EINVAL;
+
+	lp = xa_load(&secxa_xa, secxa);
+	if (!lp)
+		return -EINVAL;
+
+	*pro = lp;
+	return 0;
+}
+EXPORT_SYMBOL(secxa_get_lsmprop);
+
+/**
+ * secxa_from_lsmprop - get the secxa associated with a lsm_prop
+ * @prop: lsm_prop pointer
+ *
+ * Find the secxa associated with @prop. If there is none, create it.
+ *
+ * Returns 0, or an error if the mapping cannot be created
+ */
+int secxa_from_lsmprop(struct lsm_prop *prop)
+{
+	struct lsm_prop *lp;
+	unsigned long il;
+	unsigned int index = 0;
+	int rc;
+
+	xa_for_each(&secxa_xa, il, lp) {
+		if (!memcmp(prop, lp, sizeof(*prop)))
+			pr_info("%s found at index %lu\n", __func__, il);
+		if (!memcmp(prop, lp, sizeof(*prop)))
+			return il;
+	}
+
+	lp = kzalloc(sizeof(*lp), GFP_ATOMIC);
+	if (!lp)
+		return -ENOMEM;
+
+	rc = xa_alloc(&secxa_xa, &index, lp, xa_limit_32b, GFP_ATOMIC);
+	if (rc) {
+		kfree(lp);
+		return -EINVAL;
+	}
+	*lp = *prop;
+
+	return index;
+}
+EXPORT_SYMBOL(secxa_from_lsmprop);
+
+/**
+ * secxa_set_secmark - add LSM information to a secmark
+ * @skb: buffer with the secmark
+ * @secxa: index of the information to add
+ *
+ * If the secmark in @skb is not set, set it to @secxa.
+ */
+void secxa_set_secmark(struct sk_buff *skb, u32 secxa)
+{
+	if (!skb->secmark)
+		skb->secmark = secxa;
+}
+EXPORT_SYMBOL(secxa_set_secmark);
-- 
2.54.0

Re: [PATCH 2/7] LSM: Implement x array functions for secmarks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Newly allocated lsm_prop is published to the xarray before its contents are initialized, exposing zeroed memory to concurrent readers.
- [High] The check-then-allocate sequence (`xa_for_each` then `xa_alloc`) lacks synchronization, allowing concurrent duplicate allocations for the same property.
- [High] `secxa_from_lsmprop` returns an `int` but allows allocating indices up to `U32_MAX`, turning valid high indices into negative error codes.
- [Medium] `xa_alloc` is used without disabling softirqs, risking deadlocks if called from process context and preempted by a network softirq.
- [Low] Missing braces around an if-statement cause a redundant `memcmp` operation.
--

--- Patch [2]: [PATCH 2/7] LSM: Implement x array functions for secmarks ---
commit 71cb09431142a3a514763b3603252091420dfe8f
Author: Casey Schaufler <casey@schaufler-ca.com>
LSM: Implement x array functions for secmarks

This commit implements the infrastructure for mapping LSM properties to secmarks
using an xarray, allowing future code to store and retrieve these properties via
an allocated index.

> diff --git a/security/lsm_secxa.c b/security/lsm_secxa.c
> new file mode 100644
> index 0000000000000..5b67d8218fd25
> --- /dev/null
> +++ b/security/lsm_secxa.c
> @@ -0,0 +1,107 @@

[ ... ]

> +int secxa_from_lsmprop(struct lsm_prop *prop)
> +{
> +	struct lsm_prop *lp;
> +	unsigned long il;
> +	unsigned int index = 0;
> +	int rc;
> +
> +	xa_for_each(&secxa_xa, il, lp) {
> +		if (!memcmp(prop, lp, sizeof(*prop)))
> +			pr_info("%s found at index %lu\n", __func__, il);
> +		if (!memcmp(prop, lp, sizeof(*prop)))
> +			return il;
> +	}

[Severity: Low]
This isn't a bug, but did you intend to evaluate memcmp() twice? The first
if statement lacks braces, so the second memcmp() is evaluated on every
loop iteration. Can these be combined into a single check with braces?

[Severity: High]
Could concurrent calls to secxa_from_lsmprop() with the same property allocate
duplicate entries? Since the xa_for_each() loop locklessly checks for
existence before allocating and inserting with xa_alloc(), could two threads
both fail to find the property and insert it twice, wasting xarray indices
and leaking memory?

> +
> +	lp = kzalloc(sizeof(*lp), GFP_ATOMIC);
> +	if (!lp)
> +		return -ENOMEM;
> +
> +	rc = xa_alloc(&secxa_xa, &index, lp, xa_limit_32b, GFP_ATOMIC);

[Severity: Medium]
Is this susceptible to deadlocks if interrupted by network softirqs? The
xarray uses the standard spinlock (via XA_FLAGS_ALLOC1), but secmarks
typically bridge process and softirq contexts. Should this use
XA_FLAGS_LOCK_BH and xa_alloc_bh() to prevent a network softirq from
deadlocking on the same CPU?

[Severity: High]
Will this return negative error codes for valid high indices? The function
returns an int, but xa_limit_32b allows allocating indices up to U32_MAX.
If the index exceeds 0x7FFFFFFF, will callers incorrectly treat the returned
index as an error code? Should this use xa_limit_31b instead?

> +	if (rc) {
> +		kfree(lp);
> +		return -EINVAL;
> +	}
> +	*lp = *prop;

[Severity: High]
Could this publish a zero-initialized lsm_prop to concurrent readers? Since
xa_alloc() makes the entry visible in the xarray before the contents are
copied by *lp = *prop, is it possible for a concurrent secxa_get_lsmprop()
to retrieve an empty property?

> +
> +	return index;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=2

[PATCH 3/7] LSM: Two hooks for manipulating struct lsm_prop

[Casey Schaufler]

security_update_lsmprop() updates the property of the
specified LSM in the @dest structure with that in the @src.

security_secctx_to_lsmprop() sets the @prop field associated
with the LSM specified to the value of the passed security
context.

LSM specific implementations of these hooks to follow.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hook_defs.h |  4 ++++
 include/linux/security.h      | 17 +++++++++++++++++
 security/security.c           | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 65c9609ec207..3666d821b8a1 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -305,7 +305,11 @@ LSM_HOOK(int, 0, ismaclabel, const char *name)
 LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, struct lsm_context *cp)
 LSM_HOOK(int, -EOPNOTSUPP, lsmprop_to_secctx, struct lsm_prop *prop,
 	 struct lsm_context *cp)
+LSM_HOOK(int, -EOPNOTSUPP, update_lsmprop, struct lsm_prop *dest,
+	 struct lsm_prop *src, int lsmid)
 LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
+LSM_HOOK(int, 0, secctx_to_lsmprop, const char *secdata, u32 seclen,
+	 struct lsm_prop *prop)
 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)
diff --git a/include/linux/security.h b/include/linux/security.h
index 153e9043058f..b209d681e79a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -576,6 +576,11 @@ int security_secid_to_secctx(u32 secid, struct lsm_context *cp);
 int security_lsmprop_to_secctx(struct lsm_prop *prop, struct lsm_context *cp,
 			       int lsmid);
 int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
+int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
+			       struct lsm_prop *prop, int lsmid);
+
+int security_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
+			    int lsmid);
 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);
@@ -1581,6 +1586,12 @@ static inline int security_lsmprop_to_secctx(struct lsm_prop *prop,
 	return -EOPNOTSUPP;
 }
 
+static inline int security_update_lsmprop(struct lsm_prop *dest,
+					  struct lsm_prop *src, int lsmid)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int security_secctx_to_secid(const char *secdata,
 					   u32 seclen,
 					   u32 *secid)
@@ -1588,6 +1599,12 @@ static inline int security_secctx_to_secid(const char *secdata,
 	return -EOPNOTSUPP;
 }
 
+static inline int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
+					     struct lsm_prop *prop, int lsmid);
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void security_release_secctx(struct lsm_context *cp)
 {
 }
diff --git a/security/security.c b/security/security.c
index 71aea8fdf014..932a2eca28b3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -3965,6 +3965,13 @@ int security_lsmprop_to_secctx(struct lsm_prop *prop, struct lsm_context *cp,
 }
 EXPORT_SYMBOL(security_lsmprop_to_secctx);
 
+int security_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
+			    int lsmid)
+{
+	return call_int_hook(update_lsmprop, dest, src, lsmid);
+}
+EXPORT_SYMBOL(security_update_lsmprop);
+
 /**
  * security_secctx_to_secid() - Convert a secctx to a secid
  * @secdata: secctx
@@ -3982,6 +3989,31 @@ int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
 }
 EXPORT_SYMBOL(security_secctx_to_secid);
 
+/**
+ * security_secctx_to_lsmprop() - Convert a secctx to a lsmprop
+ * @secdata: secctx
+ * @seclen: length of secctx
+ * @prop: prop
+ * @lsmid: which LSM the context is appropriate to.
+ *
+ * Convert security context to an lsmprop.
+ *
+ * Return: Returns 0 on success, error on failure.
+ */
+int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
+			       struct lsm_prop *prop, int lsmid)
+{
+	struct lsm_static_call *scall;
+
+	lsm_for_each_hook(scall, secctx_to_lsmprop) {
+		if (lsmid != LSM_ID_UNDEF && lsmid != scall->hl->lsmid->id)
+			continue;
+		return scall->hl->hook.secctx_to_lsmprop(secdata, seclen, prop);
+	}
+	return LSM_RET_DEFAULT(secctx_to_lsmprop);
+}
+EXPORT_SYMBOL(security_secctx_to_lsmprop);
+
 /**
  * security_release_secctx() - Free a secctx buffer
  * @cp: the security context
-- 
2.54.0

Re: [PATCH 3/7] LSM: Two hooks for manipulating struct lsm_prop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] `security_secctx_to_lsmprop` silently returns success (0) when the requested LSM is not registered or no hook executes, leaving the `prop` out-parameter uninitialized.
- [Medium] `security_update_lsmprop` breaks the established dispatcher pattern for `lsmid`-targeted hooks by using `call_int_hook`, forcing the `lsmid` parameter into the hook signature and breaking the `LSM_ID_UNDEF` iteration case.
- [Low] Syntax error in the static inline stub for `security_secctx_to_lsmprop`, causing a build failure when `CONFIG_SECURITY` is disabled.
--

--- Patch [3]: [PATCH 3/7] LSM: Two hooks for manipulating struct lsm_prop ---
commit c0d20c4ec8715fc3470893661ae09affc4943cbc
Author: Casey Schaufler <casey@schaufler-ca.com>

LSM: Two hooks for manipulating struct lsm_prop

This commit introduces two new LSM hooks, security_update_lsmprop() and
security_secctx_to_lsmprop(). These hooks allow updating a destination
lsm_prop structure from a source, and converting a security context
string into an lsm_prop structure respectively.

> diff --git a/include/linux/security.h b/include/linux/security.h
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
[ ... ]
> @@ -1580,6 +1585,12 @@ static inline int security_lsmprop_to_secctx(struct lsm_prop *prop,
[ ... ]
> +static inline int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +					     struct lsm_prop *prop, int lsmid);
> +{
> +	return -EOPNOTSUPP;
> +}

[Severity: Low]
Does the trailing semicolon on the function signature here cause a build
failure when CONFIG_SECURITY is disabled?

> diff --git a/security/security.c b/security/security.c
> --- a/security/security.c
> +++ b/security/security.c
[ ... ]
> @@ -3965,6 +3965,13 @@ int security_lsmprop_to_secctx(struct lsm_prop *prop, struct lsm_context *cp,
>  }
>  EXPORT_SYMBOL(security_lsmprop_to_secctx);
>  
> +int security_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
> +			    int lsmid)
> +{
> +	return call_int_hook(update_lsmprop, dest, src, lsmid);
> +}

[Severity: Medium]
Is call_int_hook() the correct macro to use here instead of
lsm_for_each_hook()?

Because call_int_hook() aborts on the first return value that does not match
the default, if LSM_ID_UNDEF is passed to update all active LSMs, will this
loop abort immediately after the first successful LSM hook returns 0? This
might fail to update any subsequent LSMs.

Additionally, using call_int_hook() forces lsmid to be passed into the hook
signature, requiring every LSM to write boilerplate ID checks, unlike
security_secctx_to_lsmprop() which filters centrally.

[ ... ]
> @@ -3982,6 +3989,31 @@ int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
[ ... ]
> +int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +			       struct lsm_prop *prop, int lsmid)
> +{
> +	struct lsm_static_call *scall;
> +
> +	lsm_for_each_hook(scall, secctx_to_lsmprop) {
> +		if (lsmid != LSM_ID_UNDEF && lsmid != scall->hl->lsmid->id)
> +			continue;
> +		return scall->hl->hook.secctx_to_lsmprop(secdata, seclen, prop);
> +	}
> +	return LSM_RET_DEFAULT(secctx_to_lsmprop);
> +}

[Severity: High]
If the requested LSM is not registered or no hook executes, this returns
LSM_RET_DEFAULT(secctx_to_lsmprop). In include/linux/lsm_hook_defs.h, the
default return value for this hook is set to 0.

Does this mean the function will silently return success without initializing
the prop out-parameter? If callers use this API for access control, could
they end up treating uninitialized stack memory as valid security identifiers?

Should this return an error code instead, or initialize prop before returning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=3

[PATCH 4/7] SELinux: hooks for secctx_to_lsmprop and update_lsmprop

[Casey Schaufler]

Implement these hooks.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/selinux/hooks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8d6945edae7a..c05c05e71078 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6946,6 +6946,15 @@ static int selinux_ismaclabel(const char *name)
 	return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
 }
 
+static int selinux_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
+				  int lsmid)
+{
+	if (lsmid == LSM_ID_SELINUX || lsmid == LSM_ID_UNDEF)
+		dest->selinux.secid = src->selinux.secid;
+
+	return 0;
+}
+
 static int selinux_secid_to_secctx(u32 secid, struct lsm_context *cp)
 {
 	u32 seclen;
@@ -6964,6 +6973,13 @@ static int selinux_secid_to_secctx(u32 secid, struct lsm_context *cp)
 	return seclen;
 }
 
+static int selinux_secctx_to_lsmprop(const char *secdata, u32 seclen,
+				     struct lsm_prop *prop)
+{
+	return security_context_to_sid(secdata, seclen, &prop->selinux.secid,
+				       GFP_KERNEL);
+}
+
 static int selinux_lsmprop_to_secctx(struct lsm_prop *prop,
 				     struct lsm_context *cp)
 {
@@ -7694,6 +7710,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 
 	LSM_HOOK_INIT(ismaclabel, selinux_ismaclabel),
 	LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid),
+	LSM_HOOK_INIT(secctx_to_lsmprop, selinux_secctx_to_lsmprop),
 	LSM_HOOK_INIT(release_secctx, selinux_release_secctx),
 	LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx),
 	LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx),
@@ -7812,6 +7829,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(sem_alloc_security, selinux_sem_alloc_security),
 	LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx),
 	LSM_HOOK_INIT(lsmprop_to_secctx, selinux_lsmprop_to_secctx),
+	LSM_HOOK_INIT(update_lsmprop, selinux_update_lsmprop),
 	LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx),
 	LSM_HOOK_INIT(sk_alloc_security, selinux_sk_alloc_security),
 	LSM_HOOK_INIT(tun_dev_alloc_security, selinux_tun_dev_alloc_security),
-- 
2.54.0

Re: [PATCH 4/7] SELinux: hooks for secctx_to_lsmprop and update_lsmprop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The hook selinux_update_lsmprop unconditionally returns 0, short-circuiting call_int_hook and preventing other LSMs from updating their properties.
--

--- Patch [4]: [PATCH 4/7] SELinux: hooks for secctx_to_lsmprop and update_lsmprop ---
commit a4558827a3696626337a00d0c47bdfbeb141eefe
Author: Casey Schaufler <casey@schaufler-ca.com>

SELinux: hooks for secctx_to_lsmprop and update_lsmprop

This commit implements the secctx_to_lsmprop and update_lsmprop hooks for
the SELinux module. These additions allow SELinux to interact with the new
lsm_prop structures for context and property updates.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0f704380a8c81..66f32dfadf9b3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6926,6 +6926,15 @@ static int selinux_ismaclabel(const char *name)
>  	return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
>  }
>  
> +static int selinux_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
> +				  int lsmid)
> +{
> +	if (lsmid == LSM_ID_SELINUX || lsmid == LSM_ID_UNDEF)
> +		dest->selinux.secid = src->selinux.secid;
> +
> +	return 0;

[Severity: High]
Does returning 0 unconditionally here break LSM stacking?

If security_update_lsmprop() is called with an lsmid targeted at another
module (e.g., LSM_ID_SMACK), the SELinux hook will execute first. Because 0
differs from the hook's default return value (-EOPNOTSUPP), call_int_hook()
will break the loop and return 0 immediately.

The target LSM's hook will never be reached, and it will silently fail to
update its properties.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=4

[PATCH 5/7] Smack: hooks for secctx_to_lsmprop and update_lsmprop

[Casey Schaufler]

Implement these hooks.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ff115068c5c0..8e3ab61dfdd9 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4909,6 +4909,40 @@ static int smack_lsmprop_to_secctx(struct lsm_prop *prop,
 	return smack_to_secctx(prop->smack.skp, cp);
 }
 
+/**
+ * smack_secctx_to_lsmprop - add the smack label to an lsmprop
+ * @secdata: smack label
+ * @seclen: how long label is
+ * @prop: where to put the result
+ *
+ * Exists for audit and networking code.
+ */
+static int smack_secctx_to_lsmprop(const char *secdata, u32 seclen,
+				   struct lsm_prop *prop)
+{
+	prop->smack.skp = smk_find_entry(secdata);
+
+	return 0;
+}
+
+/**
+ * smack_update_lsmprop - set the smack label in an lsmprop
+ * @dest: destination properties
+ * @src: source properties
+ * @lsmid: which LSM is relevant.
+ *
+ * Set the Smack entry in the @dest if appropriate.
+ * Returns 0.
+ */
+static int smack_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
+				int lsmid)
+{
+	if (lsmid == LSM_ID_SMACK || lsmid == LSM_ID_UNDEF)
+		dest->smack.skp = src->smack.skp;
+
+	return 0;
+}
+
 /**
  * smack_secctx_to_secid - return the secid for a smack label
  * @secdata: smack label
@@ -5269,6 +5303,8 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(secid_to_secctx, smack_secid_to_secctx),
 	LSM_HOOK_INIT(lsmprop_to_secctx, smack_lsmprop_to_secctx),
 	LSM_HOOK_INIT(secctx_to_secid, smack_secctx_to_secid),
+	LSM_HOOK_INIT(secctx_to_lsmprop, smack_secctx_to_lsmprop),
+	LSM_HOOK_INIT(update_lsmprop, smack_update_lsmprop),
 	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
 	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
 	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
-- 
2.54.0

Re: [PATCH 5/7] Smack: hooks for secctx_to_lsmprop and update_lsmprop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Out-of-bounds read in `smack_secctx_to_lsmprop` due to ignored `seclen` parameter.
- [High] NULL pointer dereference when an unknown security label is provided to `smack_secctx_to_lsmprop`.
- [High] RCU locking violation when calling `smk_find_entry()` from `smack_secctx_to_lsmprop`.
--

--- Patch [5]: [PATCH 5/7] Smack: hooks for secctx_to_lsmprop and update_lsmprop ---
commit dbfa723dc5394e90f883ff953a774973bbb1759e
Author: Casey Schaufler <casey@schaufler-ca.com>

Smack: hooks for secctx_to_lsmprop and update_lsmprop

Implement these hooks.

> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 3f9ae05039a28..cd9f5765979ff 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
[ ... ]
> @@ -4914,6 +4914,40 @@ static int smack_lsmprop_to_secctx(struct lsm_prop *prop,
>  	return smack_to_secctx(prop->smack.skp, cp);
>  }
>  
> +/**
> + * smack_secctx_to_lsmprop - add the smack label to an lsmprop
> + * @secdata: smack label
> + * @seclen: how long label is
> + * @prop: where to put the result
> + *
> + * Exists for audit and networking code.
> + */
> +static int smack_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +				   struct lsm_prop *prop)
> +{
> +	prop->smack.skp = smk_find_entry(secdata);

[Severity: High]
Does this code ignore the seclen parameter? If secdata is provided by
networking code or audit without null termination, passing it directly to
smk_find_entry() will cause an out-of-bounds read:

security/smack/smack_access.c:smk_find_entry() {
	...
	hash = full_name_hash(NULL, string, strlen(string));
	...
}

Should this code respect seclen or ensure secdata is null-terminated before
the search?


[Severity: High]
Is it safe to call smk_find_entry() here without holding the RCU read lock?

Looking at smk_find_entry() in security/smack/smack_access.c, it traverses the
hash list using RCU:

	hlist_for_each_entry_rcu(skp, head, smk_hashed) {
		...
	}

Without an rcu_read_lock() wrapping this call, could this trigger a lockdep
warning or read malformed pointers during a concurrent insertion?

> +
> +	return 0;

[Severity: High]
If smk_find_entry() fails to find a matching label, it returns NULL. By
returning 0 (success) here, we leave prop->smack.skp as NULL.

When this property is later passed to smack_lsmprop_to_secctx(), it will
delegate to smack_to_secctx():

security/smack/smack_lsm.c:smack_to_secctx() {
	int len = strlen(skp->smk_known);
	...
}

Will this cause a deterministic NULL pointer dereference? Should this
function return an error like -EINVAL when the label is not found?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=5

[PATCH 6/7] Apparmor: hooks for secctx_to_lsmprop and update_lsmprop

[Casey Schaufler]

Implement these hooks.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/apparmor/include/secid.h |  4 ++++
 security/apparmor/lsm.c           |  2 ++
 security/apparmor/secid.c         | 23 +++++++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h
index 6025d3849cf8..b290ae8f9b01 100644
--- a/security/apparmor/include/secid.h
+++ b/security/apparmor/include/secid.h
@@ -28,6 +28,10 @@ struct aa_label *aa_secid_to_label(u32 secid);
 int apparmor_secid_to_secctx(u32 secid, struct lsm_context *cp);
 int apparmor_lsmprop_to_secctx(struct lsm_prop *prop, struct lsm_context *cp);
 int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
+int apparmor_secctx_to_lsmprop(const char *secdata, u32 seclen,
+			       struct lsm_prop *prop);
+int apparmor_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
+			    int lsmid);
 void apparmor_release_secctx(struct lsm_context *cp);
 
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 88d12e89d115..1f304b88eaf9 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1766,6 +1766,8 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(secid_to_secctx, apparmor_secid_to_secctx),
 	LSM_HOOK_INIT(lsmprop_to_secctx, apparmor_lsmprop_to_secctx),
 	LSM_HOOK_INIT(secctx_to_secid, apparmor_secctx_to_secid),
+	LSM_HOOK_INIT(secctx_to_lsmprop, apparmor_secctx_to_lsmprop),
+	LSM_HOOK_INIT(update_lsmprop, apparmor_update_lsmprop),
 	LSM_HOOK_INIT(release_secctx, apparmor_release_secctx),
 
 #ifdef CONFIG_IO_URING
diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
index 28caf66b9033..51f3e4ab053e 100644
--- a/security/apparmor/secid.c
+++ b/security/apparmor/secid.c
@@ -106,6 +106,29 @@ int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
 	return 0;
 }
 
+int apparmor_secctx_to_lsmprop(const char *secdata, u32 seclen,
+			       struct lsm_prop *prop)
+{
+	struct aa_label *label;
+
+	label = aa_label_strn_parse(&root_ns->unconfined->label, secdata,
+				    seclen, GFP_KERNEL, false, false);
+	if (IS_ERR(label))
+		return PTR_ERR(label);
+	prop->apparmor.label = label;
+
+	return 0;
+}
+
+int apparmor_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
+			    int lsmid)
+{
+	if (lsmid == LSM_ID_APPARMOR || lsmid == LSM_ID_UNDEF)
+		dest->apparmor.label = src->apparmor.label;
+
+	return 0;
+}
+
 void apparmor_release_secctx(struct lsm_context *cp)
 {
 	if (cp->id == LSM_ID_APPARMOR) {
-- 
2.54.0

Re: [PATCH 6/7] Apparmor: hooks for secctx_to_lsmprop and update_lsmprop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] apparmor_secctx_to_lsmprop leaks a reference count on the parsed aa_label, exposing a fundamental design conflict with lsm_prop.

Pre-existing issues:
- [High] Preexisting memory leak in apparmor_secctx_to_secid when extracting the secid from a parsed aa_label.
--

--- Patch [6]: [PATCH 6/7] Apparmor: hooks for secctx_to_lsmprop and update_lsmprop ---
commit 81357db42de4a31531a31239f8d75a01c866a80e
Author: Casey Schaufler <casey@schaufler-ca.com>

Apparmor: hooks for secctx_to_lsmprop and update_lsmprop

This commit implements the secctx_to_lsmprop and update_lsmprop LSM
hooks for AppArmor. It adds the capability to convert a security
context string into a struct lsm_prop containing an AppArmor label.

> diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
> index 28caf66b90335..51f3e4ab053e5 100644
> --- a/security/apparmor/secid.c
> +++ b/security/apparmor/secid.c
> @@ -106,6 +106,29 @@ int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does apparmor_secctx_to_secid leak the
label reference?

When aa_label_strn_parse is called, it allocates or refcounts an aa_label.
The secid is extracted, but it appears aa_put_label is never called
before returning, causing a memory leak that could be triggered by an
unprivileged user with CAP_NET_ADMIN in a user or network namespace.

> +int apparmor_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +			       struct lsm_prop *prop)
> +{
> +	struct aa_label *label;
> +
> +	label = aa_label_strn_parse(&root_ns->unconfined->label, secdata,
> +				    seclen, GFP_KERNEL, false, false);
> +	if (IS_ERR(label))
> +		return PTR_ERR(label);
> +	prop->apparmor.label = label;
> +
> +	return 0;
> +}

[Severity: High]
Does this code leak the refcount on the parsed aa_label?

Similar to the above issue, aa_label_strn_parse returns a refcounted label,
but struct lsm_prop does not seem to have a mechanism to release it.

Furthermore, since struct lsm_prop acts as an unmanaged container (as seen
in apparmor_update_lsmprop performing a raw pointer copy), if we were
to drop the reference here, wouldn't prop->apparmor.label become a dangling
pointer, leading to a use-after-free?

> +int apparmor_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
> +			    int lsmid)
> +{
> +	if (lsmid == LSM_ID_APPARMOR || lsmid == LSM_ID_UNDEF)
> +		dest->apparmor.label = src->apparmor.label;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=6

[PATCH 7/7] net, lsm: Change skb secmarks to x-array indexes

[Casey Schaufler]

Maintain a xarray of lsm_prop structures which represent the
LSM security information passed via skb->secmark. Pass the xarray
index of the appropriate lsm_prop (the secxa) instead of an LSM
specific secid. Allow multiple LSMs to specify their components
in xarray entries, or create new entries as necessary.

Change uses of security_secctx_to_secid() to security_secctx_to_lsmprop()
in the netfilter and iptables code. Change security_secmark_relabel_packet()
to accept an lsm_prop pointer rather than a secid. Change secxa_set_secmark()
to update and create new entries as necessary.

Update the SELinux, Smack and AppArmor hooks that use secmarks to
expect a secxa xarray index instead of a secid.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hook_defs.h    |  2 +-
 include/linux/security.h         |  4 +-
 net/netfilter/nfnetlink_queue.c  | 12 +++++-
 net/netfilter/nft_meta.c         | 11 +++--
 net/netfilter/xt_SECMARK.c       | 12 ++++--
 security/apparmor/net.c          |  8 +++-
 security/lsm_secxa.c             | 18 ++++++--
 security/security.c              |  6 +--
 security/selinux/hooks.c         | 71 +++++++++++++++++++++++++++-----
 security/smack/smack_lsm.c       | 10 ++++-
 security/smack/smack_netfilter.c |  8 ++--
 11 files changed, 127 insertions(+), 35 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 3666d821b8a1..7ba4547daded 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -371,7 +371,7 @@ LSM_HOOK(void, LSM_RET_VOID, inet_csk_clone, struct sock *newsk,
 	 const struct request_sock *req)
 LSM_HOOK(void, LSM_RET_VOID, inet_conn_established, struct sock *sk,
 	 struct sk_buff *skb)
-LSM_HOOK(int, 0, secmark_relabel_packet, u32 secid)
+LSM_HOOK(int, 0, secmark_relabel_packet, struct lsm_prop *prop)
 LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_inc, void)
 LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_dec, void)
 LSM_HOOK(void, LSM_RET_VOID, req_classify_flow, const struct request_sock *req,
diff --git a/include/linux/security.h b/include/linux/security.h
index b209d681e79a..844fbef4a6a8 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1716,7 +1716,7 @@ void security_inet_csk_clone(struct sock *newsk,
 			const struct request_sock *req);
 void security_inet_conn_established(struct sock *sk,
 			struct sk_buff *skb);
-int security_secmark_relabel_packet(u32 secid);
+int security_secmark_relabel_packet(struct lsm_prop *prop);
 void security_secmark_refcount_inc(void);
 void security_secmark_refcount_dec(void);
 int security_tun_dev_alloc_security(void **security);
@@ -1899,7 +1899,7 @@ static inline void security_inet_conn_established(struct sock *sk,
 {
 }
 
-static inline int security_secmark_relabel_packet(u32 secid)
+static inline int security_secmark_relabel_packet(struct lsm_prop *prop)
 {
 	return 0;
 }
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index b8aaf39cb4d8..ebab037edc6b 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -32,6 +32,7 @@
 #include <linux/cgroup-defs.h>
 #include <linux/rhashtable.h>
 #include <linux/jhash.h>
+#include <linux/lsm_secxa.h>
 #include <net/gso.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
@@ -606,8 +607,15 @@ static int nfqnl_get_sk_secctx(struct sk_buff *skb, struct lsm_context *ctx)
 {
 	int seclen = 0;
 #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
-	if (skb->secmark)
-		seclen = security_secid_to_secctx(skb->secmark, ctx);
+	struct lsm_prop *prop;
+	int rc;
+
+	if (skb->secmark) {
+		rc = secxa_get_lsmprop(&prop, skb->secmark);
+		if (rc)
+			return 0;
+		seclen = security_lsmprop_to_secctx(prop, ctx, LSM_ID_UNDEF);
+	}
 #endif
 	return seclen;
 }
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index bd0f7a0931f4..24535c45dab3 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -927,17 +927,20 @@ static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
 
 static int nft_secmark_compute_secid(struct nft_secmark *priv)
 {
+	struct lsm_prop tmp_prop;
 	u32 tmp_secid = 0;
 	int err;
 
-	err = security_secctx_to_secid(priv->ctx, strlen(priv->ctx), &tmp_secid);
+	err = security_secctx_to_lsmprop(priv->ctx, strlen(priv->ctx),
+					 &tmp_prop, LSM_ID_UNDEF);
 	if (err)
 		return err;
 
-	if (!tmp_secid)
-		return -ENOENT;
+	tmp_secid = secxa_from_lsmprop(&tmp_prop);
+	if (tmp_secid < 0)
+		return tmp_secid;
 
-	err = security_secmark_relabel_packet(tmp_secid);
+	err = security_secmark_relabel_packet(&tmp_prop);
 	if (err)
 		return err;
 
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index ea67aa92ddc2..5a7b83c67430 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -43,13 +43,15 @@ secmark_tg(struct sk_buff *skb, const struct xt_secmark_target_info_v1 *info)
 
 static int checkentry_lsm(struct xt_secmark_target_info_v1 *info)
 {
+	struct lsm_prop prop;
 	int err;
 
 	info->secctx[SECMARK_SECCTX_MAX - 1] = '\0';
 	info->secid = 0;
 
-	err = security_secctx_to_secid(info->secctx, strlen(info->secctx),
-				       &info->secid);
+	err = security_secctx_to_lsmprop(info->secctx, strlen(info->secctx),
+					 &prop, LSM_ID_UNDEF);
+
 	if (err) {
 		if (err == -EINVAL)
 			pr_info_ratelimited("invalid security context \'%s\'\n",
@@ -57,18 +59,20 @@ static int checkentry_lsm(struct xt_secmark_target_info_v1 *info)
 		return err;
 	}
 
-	if (!info->secid) {
+	if (!lsmprop_is_set(&prop)) {
 		pr_info_ratelimited("unable to map security context \'%s\'\n",
 				    info->secctx);
 		return -ENOENT;
 	}
 
-	err = security_secmark_relabel_packet(info->secid);
+	err = security_secmark_relabel_packet(&prop);
 	if (err) {
 		pr_info_ratelimited("unable to obtain relabeling permission\n");
 		return err;
 	}
 
+	info->secid = secxa_from_lsmprop(&prop);
+
 	security_secmark_refcount_inc();
 	return 0;
 }
diff --git a/security/apparmor/net.c b/security/apparmor/net.c
index cf590dd08540..e26e15c2d947 100644
--- a/security/apparmor/net.c
+++ b/security/apparmor/net.c
@@ -8,6 +8,7 @@
  * Copyright 2009-2017 Canonical Ltd.
  */
 
+#include <linux/lsm_secxa.h>
 #include "include/af_unix.h"
 #include "include/apparmor.h"
 #include "include/audit.h"
@@ -365,12 +366,17 @@ static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid,
 			   struct apparmor_audit_data *ad)
 {
 	int i, ret;
+	struct lsm_prop *prop;
 	struct aa_perms perms = { };
 	struct aa_ruleset *rules = profile->label.rules[0];
 
 	if (rules->secmark_count == 0)
 		return 0;
 
+	ret = secxa_get_lsmprop(&prop, secid);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < rules->secmark_count; i++) {
 		if (!rules->secmark[i].secid) {
 			ret = apparmor_secmark_init(&rules->secmark[i]);
@@ -378,7 +384,7 @@ static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid,
 				return ret;
 		}
 
-		if (rules->secmark[i].secid == secid ||
+		if (rules->secmark[i].secid == prop->apparmor.label->secid ||
 		    rules->secmark[i].secid == AA_SECID_WILDCARD) {
 			if (rules->secmark[i].deny)
 				perms.deny = ALL_PERMS_MASK;
diff --git a/security/lsm_secxa.c b/security/lsm_secxa.c
index 5b67d8218fd2..2015dab01bdf 100644
--- a/security/lsm_secxa.c
+++ b/security/lsm_secxa.c
@@ -71,8 +71,6 @@ int secxa_from_lsmprop(struct lsm_prop *prop)
 	int rc;
 
 	xa_for_each(&secxa_xa, il, lp) {
-		if (!memcmp(prop, lp, sizeof(*prop)))
-			pr_info("%s found at index %lu\n", __func__, il);
 		if (!memcmp(prop, lp, sizeof(*prop)))
 			return il;
 	}
@@ -101,7 +99,21 @@ EXPORT_SYMBOL(secxa_from_lsmprop);
  */
 void secxa_set_secmark(struct sk_buff *skb, u32 secxa)
 {
-	if (!skb->secmark)
+	struct lsm_prop *olp;
+	struct lsm_prop *nlp;
+	struct lsm_prop prop;
+
+	if (!skb->secmark) {
 		skb->secmark = secxa;
+		return;
+	}
+
+	olp = xa_load(&secxa_xa, skb->secmark);
+	nlp = xa_load(&secxa_xa, secxa);
+
+	prop = *olp;
+	security_update_lsmprop(&prop, nlp, LSM_ID_UNDEF);
+
+	skb->secmark = secxa_from_lsmprop(&prop);
 }
 EXPORT_SYMBOL(secxa_set_secmark);
diff --git a/security/security.c b/security/security.c
index 932a2eca28b3..059cf0a97d2c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4646,15 +4646,15 @@ EXPORT_SYMBOL(security_inet_conn_established);
 
 /**
  * security_secmark_relabel_packet() - Check if setting a secmark is allowed
- * @secid: new secmark value
+ * @lsmprop: new secmark value
  *
  * Check if the process should be allowed to relabel packets to @secid.
  *
  * Return: Returns 0 if permission is granted.
  */
-int security_secmark_relabel_packet(u32 secid)
+int security_secmark_relabel_packet(struct lsm_prop *prop)
 {
-	return call_int_hook(secmark_relabel_packet, secid);
+	return call_int_hook(secmark_relabel_packet, prop);
 }
 EXPORT_SYMBOL(security_secmark_relabel_packet);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c05c05e71078..55d7679f3403 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -94,6 +94,7 @@
 #include <linux/io_uring/cmd.h>
 #include <uapi/linux/lsm.h>
 #include <linux/memfd.h>
+#include <linux/lsm_secxa.h>
 
 #include "initcalls.h"
 #include "avc.h"
@@ -5414,7 +5415,16 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 		return err;
 
 	if (selinux_secmark_enabled()) {
-		err = avc_has_perm(sk_sid, skb->secmark, SECCLASS_PACKET,
+		struct lsm_prop *prop;
+		u32 secmark = 0;
+
+		if (skb->secmark) {
+			err = secxa_get_lsmprop(&prop, skb->secmark);
+			if (!err)
+				secmark = prop->selinux.secid;
+		}
+
+		err = avc_has_perm(sk_sid, secmark, SECCLASS_PACKET,
 				   PACKET__RECV, &ad);
 		if (err)
 			return err;
@@ -5483,7 +5493,15 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	}
 
 	if (secmark_active) {
-		err = avc_has_perm(sk_sid, skb->secmark, SECCLASS_PACKET,
+		struct lsm_prop *prop;
+		u32 secmark = 0;
+
+		if (skb->secmark) {
+			err = secxa_get_lsmprop(&prop, skb->secmark);
+			if (!err)
+				secmark = prop->selinux.secid;
+		}
+		err = avc_has_perm(sk_sid, secmark, SECCLASS_PACKET,
 				   PACKET__RECV, &ad);
 		if (err)
 			return err;
@@ -5885,10 +5903,10 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
 	selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
 }
 
-static int selinux_secmark_relabel_packet(u32 sid)
+static int selinux_secmark_relabel_packet(struct lsm_prop *lsmprop)
 {
-	return avc_has_perm(current_sid(), sid, SECCLASS_PACKET, PACKET__RELABELTO,
-			    NULL);
+	return avc_has_perm(current_sid(), lsmprop->selinux.secid,
+			    SECCLASS_PACKET, PACKET__RELABELTO, NULL);
 }
 
 static void selinux_secmark_refcount_inc(void)
@@ -6016,10 +6034,21 @@ static unsigned int selinux_ip_forward(void *priv, struct sk_buff *skb,
 		}
 	}
 
-	if (secmark_active)
-		if (avc_has_perm(peer_sid, skb->secmark,
+	if (secmark_active) {
+		struct lsm_prop *prop;
+		u32 secmark = 0;
+		int err;
+
+		if (skb->secmark) {
+			err = secxa_get_lsmprop(&prop, skb->secmark);
+			if (!err)
+				secmark = prop->selinux.secid;
+		}
+
+		if (avc_has_perm(peer_sid, secmark,
 				 SECCLASS_PACKET, PACKET__FORWARD_IN, &ad))
 			return NF_DROP;
+	}
 
 	if (netlbl_enabled())
 		/* we do this in the FORWARD path and not the POST_ROUTING
@@ -6093,10 +6122,20 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
 	if (selinux_parse_skb(skb, &ad, NULL, 0, &proto))
 		return NF_DROP;
 
-	if (selinux_secmark_enabled())
-		if (avc_has_perm(sksec->sid, skb->secmark,
+	if (selinux_secmark_enabled()) {
+		struct lsm_prop *prop;
+		u32 secmark = 0;
+		int err;
+
+		if (skb->secmark) {
+			err = secxa_get_lsmprop(&prop, skb->secmark);
+			if (!err)
+				secmark = prop->selinux.secid;
+		}
+		if (avc_has_perm(sksec->sid, secmark,
 				 SECCLASS_PACKET, PACKET__SEND, &ad))
 			return NF_DROP_ERR(-ECONNREFUSED);
+	}
 
 	if (selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto))
 		return NF_DROP_ERR(-ECONNREFUSED);
@@ -6215,10 +6254,20 @@ static unsigned int selinux_ip_postroute(void *priv,
 	if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
 		return NF_DROP;
 
-	if (secmark_active)
-		if (avc_has_perm(peer_sid, skb->secmark,
+	if (secmark_active) {
+		struct lsm_prop *prop;
+		u32 secmark = 0;
+		int err;
+
+		if (skb->secmark) {
+			err = secxa_get_lsmprop(&prop, skb->secmark);
+			if (!err)
+				secmark = prop->selinux.secid;
+		}
+		if (avc_has_perm(peer_sid, secmark,
 				 SECCLASS_PACKET, secmark_perm, &ad))
 			return NF_DROP_ERR(-ECONNREFUSED);
+	}
 
 	if (peerlbl_active) {
 		u32 if_sid;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 8e3ab61dfdd9..38bd97c89179 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -42,6 +42,7 @@
 #include <linux/fs_context.h>
 #include <linux/fs_parser.h>
 #include <linux/watch_queue.h>
+#include <linux/lsm_secxa.h>
 #include <linux/io_uring/cmd.h>
 #include <uapi/linux/lsm.h>
 #include "smack.h"
@@ -4189,10 +4190,17 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
 #ifdef CONFIG_NETWORK_SECMARK
 static struct smack_known *smack_from_skb(struct sk_buff *skb)
 {
+	struct lsm_prop *prop;
+	int rc;
+
 	if (skb == NULL || skb->secmark == 0)
 		return NULL;
 
-	return smack_from_secid(skb->secmark);
+	rc = secxa_get_lsmprop(&prop, skb->secmark);
+	if (prop)
+		return prop->smack.skp;
+
+	return NULL;
 }
 #else
 static inline struct smack_known *smack_from_skb(struct sk_buff *skb)
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index b363c42f252e..5270b55eaacf 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -24,13 +24,15 @@ static unsigned int smack_ip_output(void *priv,
 				    const struct nf_hook_state *state)
 {
 	struct sock *sk = skb_to_full_sk(skb);
+	struct lsm_prop prop = { };
 	struct socket_smack *ssp;
-	struct smack_known *skp;
+	int secxa;
 
 	if (sk) {
 		ssp = smack_sock(sk);
-		skp = ssp->smk_out;
-		secxa_set_secmark(skb, skp->smk_secid);
+		prop.smack.skp = ssp->smk_out;
+		secxa = secxa_from_lsmprop(&prop);
+		secxa_set_secmark(skb, secxa);
 	}
 
 	return NF_ACCEPT;
-- 
2.54.0

Re: [PATCH 7/7] net, lsm: Change skb secmarks to x-array indexes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [Critical] Missing NULL checks on the results of `xa_load()` in `secxa_set_secmark()` causes unconditional kernel panics.
- [Critical] In `smack_from_skb()`, failure from `secxa_get_lsmprop()` leaves a stack pointer uninitialized, leading to a wild pointer dereference.
- [High] Uninitialized stack memory causes XArray deduplication failures and leaks kernel stack data.
- [High] Negative error codes from `secxa_from_lsmprop()` are truncated to unsigned 32-bit integers, bypassing error checks.
- [High] The XArray is allowed to allocate index 0, breaking the `skb->secmark == 0` universal sentinel for unlabeled packets.
- [High] A newly allocated `struct lsm_prop` is inserted into the global XArray before its contents are initialized, leading to a race condition that corrupts packet labels.
- [High] AppArmor unconditionally dereferences `prop->apparmor.label`, leading to a crash if the secmark was created by another LSM.
- [Medium] The XArray allocation limit `xa_limit_32b` exceeds the maximum positive value representable by the `int` return type.
- [Medium] Missing synchronization around the read-allocate-insert sequence in `secxa_from_lsmprop()` allows concurrent threads to insert duplicate entries.
- [Medium] SELinux hooks evaluate packets as unlabeled when `secxa_get_lsmprop()` fails, potentially bypassing security policies.
--

--- Patch [7]: [PATCH 7/7] net, lsm: Change skb secmarks to x-array indexes ---
commit 4d10524e4c6ddab86481df7f6394707d0a6f8278
Author: Casey Schaufler <casey@schaufler-ca.com>

net, lsm: Change skb secmarks to x-array indexes

This commit shifts the skb secmark infrastructure to maintain an xarray of
lsm_prop structures. It replaces the LSM-specific secid with an xarray
index representing the security information.

> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index bd9a9377919cf..0ce662ec3de06 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -926,17 +926,20 @@ static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
>  
>  static int nft_secmark_compute_secid(struct nft_secmark *priv)
>  {
> +	struct lsm_prop tmp_prop;
>  	u32 tmp_secid = 0;
>  	int err;
>  
> -	err = security_secctx_to_secid(priv->ctx, strlen(priv->ctx), &tmp_secid);
> +	err = security_secctx_to_lsmprop(priv->ctx, strlen(priv->ctx),
> +					 &tmp_prop, LSM_ID_UNDEF);

[Severity: High]
Does this leave the tmp_prop structure partially uninitialized on the stack?

Since security_secctx_to_lsmprop() only initializes fields for the active
LSM, the rest of the struct might contain stack garbage. When passed to
secxa_from_lsmprop(), memcmp() is used on the entire struct, which can fail
to match due to garbage bytes, breaking XArray deduplication and leaking
kernel stack data.

Is it necessary to initialize this with an empty struct assignment?

>  	if (err)
>  		return err;
>  
> -	if (!tmp_secid)
> -		return -ENOENT;
> +	tmp_secid = secxa_from_lsmprop(&tmp_prop);
> +	if (tmp_secid < 0)
> +		return tmp_secid;

[Severity: High]
Can the tmp_secid < 0 check ever be true here?

Since tmp_secid is declared as a u32, a negative error code from
secxa_from_lsmprop() (like -ENOMEM) will be truncated to a large unsigned
integer, bypassing the error check. This could store an invalid secmark that
causes guaranteed lookup failures and crashes later.

> diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
> index ea67aa92ddc2f..5a7b83c674305 100644
> --- a/net/netfilter/xt_SECMARK.c
> +++ b/net/netfilter/xt_SECMARK.c
> @@ -43,13 +43,15 @@ secmark_tg(struct sk_buff *skb, const struct xt_secmark_target_info_v1 *info)
>  
>  static int checkentry_lsm(struct xt_secmark_target_info_v1 *info)
>  {
> +	struct lsm_prop prop;
>  	int err;
>  
>  	info->secctx[SECMARK_SECCTX_MAX - 1] = '\0';
>  	info->secid = 0;
>  
> -	err = security_secctx_to_secid(info->secctx, strlen(info->secctx),
> -				       &info->secid);
> +	err = security_secctx_to_lsmprop(info->secctx, strlen(info->secctx),
> +					 &prop, LSM_ID_UNDEF);
> +

[Severity: High]
Similar to the issue in nft_meta.c, does this leave the prop variable
partially uninitialized on the stack?

[ ... ]

> -	err = security_secmark_relabel_packet(info->secid);
> +	err = security_secmark_relabel_packet(&prop);
>  	if (err) {
>  		pr_info_ratelimited("unable to obtain relabeling permission\n");
>  		return err;
>  	}
>  
> +	info->secid = secxa_from_lsmprop(&prop);

[Severity: High]
Does this directly assign a potential negative error code to info->secid
without any error checking?

> diff --git a/security/apparmor/net.c b/security/apparmor/net.c
> index 44c04102062f3..1d33734d15594 100644
> --- a/security/apparmor/net.c
> +++ b/security/apparmor/net.c
> @@ -362,12 +363,17 @@ static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid,
>  			   struct apparmor_audit_data *ad)
>  {
>  	int i, ret;
> +	struct lsm_prop *prop;
>  	struct aa_perms perms = { };
>  	struct aa_ruleset *rules = profile->label.rules[0];
>  
>  	if (rules->secmark_count == 0)
>  		return 0;
>  
> +	ret = secxa_get_lsmprop(&prop, secid);
> +	if (ret)
> +		return ret;
> +
>  	for (i = 0; i < rules->secmark_count; i++) {
>  		if (!rules->secmark[i].secid) {
>  			ret = apparmor_secmark_init(&rules->secmark[i]);
> @@ -375,7 +381,7 @@ static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid,
>  				return ret;
>  		}
>  
> -		if (rules->secmark[i].secid == secid ||
> +		if (rules->secmark[i].secid == prop->apparmor.label->secid ||

[Severity: High]
Does this unconditionally dereference prop->apparmor.label?

If the packet's secmark was generated by a different LSM (like SELinux
setting iptables rules), the apparmor.label field might be NULL. Could this
cause a NULL pointer dereference crash?

> diff --git a/security/lsm_secxa.c b/security/lsm_secxa.c
> index 5b67d8218fd25..2015dab01bdf4 100644
> --- a/security/lsm_secxa.c
> +++ b/security/lsm_secxa.c
> @@ -71,8 +71,6 @@ int secxa_from_lsmprop(struct lsm_prop *prop)
>  	int rc;
>  
>  	xa_for_each(&secxa_xa, il, lp) {
> -		if (!memcmp(prop, lp, sizeof(*prop)))
> -			pr_info("%s found at index %lu\n", __func__, il);
>  		if (!memcmp(prop, lp, sizeof(*prop)))
>  			return il;
>  	}
> @@ -101,7 +99,21 @@ EXPORT_SYMBOL(secxa_from_lsmprop);

[Severity: Medium]
Is this read-allocate-insert sequence safe from concurrent updates?

Since there is no lock protecting this check-then-act block, multiple
threads processing packets with identical security contexts could
simultaneously fail the search phase and insert duplicate entries.

> +	rc = xa_alloc(&secxa_xa, &index, lp, xa_limit_32b, GFP_ATOMIC);

[Severity: High]
Could xa_alloc assign index 0 here?

The networking core across subsystems uses skb->secmark == 0 as a universal
sentinel for unlabeled packets. If index 0 is allocated to a valid context,
will those labeled packets be incorrectly evaluated as unlabeled?

[Severity: Medium]
Because xa_limit_32b allows indices up to U32_MAX, could an index larger
than INT_MAX be returned here as a negative integer, causing callers to
treat it as an error code since the function returns an int?

> +	if (rc) {
> +		kfree(lp);
> +		return -EINVAL;
> +	}
> +	*lp = *prop;
> +
> +	return index;
> +}

[Severity: High]
Does this introduce a race condition by assigning *lp = *prop after
xa_alloc() has made the pointer globally visible in the XArray?

Another thread doing a lookup might see the fully zeroed memory and use it
to label its packets before the memory is fully initialized here, leading to
incorrect policy application.

>  void secxa_set_secmark(struct sk_buff *skb, u32 secxa)
>  {
> -	if (!skb->secmark)
> +	struct lsm_prop *olp;
> +	struct lsm_prop *nlp;
> +	struct lsm_prop prop;
> +
> +	if (!skb->secmark) {
>  		skb->secmark = secxa;
> +		return;
> +	}
> +
> +	olp = xa_load(&secxa_xa, skb->secmark);
> +	nlp = xa_load(&secxa_xa, secxa);
> +
> +	prop = *olp;
> +	security_update_lsmprop(&prop, nlp, LSM_ID_UNDEF);

[Severity: Critical]
Does this safely handle xa_load() returning NULL?

If the secmark index does not exist, olp or nlp will be NULL, and the
subsequent prop = *olp assignment or security_update_lsmprop() call would
cause a panic.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 66f32dfadf9b3..1739dd2eb0779 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5996,10 +6014,21 @@ static unsigned int selinux_ip_forward(void *priv, struct sk_buff *skb,
>  		}
>  	}
>  
> -	if (secmark_active)
> -		if (avc_has_perm(peer_sid, skb->secmark,
> +	if (secmark_active) {
> +		struct lsm_prop *prop;
> +		u32 secmark = 0;
> +		int err;
> +
> +		if (skb->secmark) {
> +			err = secxa_get_lsmprop(&prop, skb->secmark);
> +			if (!err)
> +				secmark = prop->selinux.secid;
> +		}
> +
> +		if (avc_has_perm(peer_sid, secmark,
>  				 SECCLASS_PACKET, PACKET__FORWARD_IN, &ad))
>  			return NF_DROP;
> +	}

[Severity: Medium]
If secxa_get_lsmprop() fails, secmark retains its default value of 0.
Will this cause the packet to be evaluated as unlabeled instead of being
dropped? This looks like it might act as a fallback that could bypass
restrictive policies. Similar logic applies to selinux_ip_postroute().

> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index cd9f5765979ff..2eddd47556c79 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4194,10 +4195,17 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
>  #ifdef CONFIG_NETWORK_SECMARK
>  static struct smack_known *smack_from_skb(struct sk_buff *skb)
>  {
> +	struct lsm_prop *prop;
> +	int rc;
> +
>  	if (skb == NULL || skb->secmark == 0)
>  		return NULL;
>  
> -	return smack_from_secid(skb->secmark);
> +	rc = secxa_get_lsmprop(&prop, skb->secmark);
> +	if (prop)
> +		return prop->smack.skp;

[Severity: Critical]
If secxa_get_lsmprop() returns an error, could prop remain uninitialized on
the stack?

The if (prop) check evaluates this uninitialized stack memory, which might
evaluate to true and lead to a wild pointer dereference when accessing
prop->smack.skp.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813204854.19211-1-casey@schaufler-ca.com?part=7