[PATCH 00/19] smack: clean up xattr handling

[PATCH 00/19] smack: clean up xattr handling

[Konstantin Andreev]

A set of minor bug fixes and optimizations in Smack xattr handling.
Logically independent, but with the code dependencies.

The patch set applies on top of:
https://github.com/cschaufler/smack-next/commits/next
commit 6ddd169d0288

Konstantin Andreev (19):
  smack: fix bug: changing Smack xattrs requires cap_sys_admin
  smack: fix bug: changing Smack xattrs requires cap_mac_override
  smack: fix bug: setting label-containing xattrs silently ignores input garbage
  smack: stop polling other LSMs & VFS to getxattr() unsupported SMACK64IPIN/OUT
  smack: restrict getxattr() SMACK64TRANSMUTE to directories
  smack: fix bug: getxattr() returns invalid SMACK64EXEC/MMAP
  smack: deduplicate task label validation
  smack: smack_inode_setsecurity: prevent setting SMACK64EXEC/MMAP in other LSMs
  smack: smack_inode_setsecurity: prevent setting SMACK64IPIN/OUT in other LSMs
  smack: fix bug: smack_inode_setsecurity() imports alien xattrs as labels
  smack: fix bug: smack_inode_setsecurity() false EINVAL for alien xattrs
  smack: restrict setxattr() SMACK64IPIN/IPOUT to sockets
  smack: restrict setxattr() SMACK64EXEC/MMAP to regular files
  smack: return EOPNOTSUPP for setxattr() unsupported SMACK64(TRANSMUTE)
  smack: smack_inode_setsecurity(): skip checks for SMACK64TRANSMUTE
  smack: smack_inode_notifysecctx(): reject invalid labels
  smack: smack_inode_post_setxattr(): find label instead of import
  smack: smack_inode_setsecurity(): find label instead of import
  smack: deduplicate strcmp(name, XATTR_{,NAME_}SMACK*)

 Documentation/admin-guide/LSM/Smack.rst |   3 +-
 security/smack/smack.h                  |   2 +
 security/smack/smack_access.c           |  22 +-
 security/smack/smack_lsm.c              | 492 +++++++++++++++---------
 4 files changed, 324 insertions(+), 195 deletions(-)

-- 
2.43.0

[PATCH 01/19] smack: fix bug: changing Smack xattrs requires cap_sys_admin

[Konstantin Andreev]

[1] introduced a new LSM hook, inode_xattr_skipcap.
This hook is intended to identify security xattrs
for which the LSM takes responsibility for access control.

However, the Smack implementation, smack_inode_xattr_skipcap(),
has never worked as intended. It mistakenly does not recognize
the security.SMACK64* xattrs it owns.

As a result, Smack does not inform the common security layer
(security/security.c) that Smack is responsible for
security.SMACK64* xattrs. Consequently, the generic access
control functions (cap_inode_removexattr, cap_inode_setxattr)
are invoked, and they require cap_sys_admin to be effective.

This change corrects smack_inode_xattr_skipcap(),
allowing Smack xattrs to skip cap_inode_*xattr() calls.

[1] 2024-05-02, Paul Moore
Fixes: 61df7b828204 ("lsm: fixup the inode xattr capability handling")
Link: https://lore.kernel.org/linux-security-module/20240503005850.466144-2-paul@paul-moore.com/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index adf1c542d213..42fdac05d32d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1312,7 +1312,7 @@ static int smack_inode_getattr(const struct path *path)
  */
 static int smack_inode_xattr_skipcap(const char *name)
 {
-	if (strncmp(name, XATTR_SMACK_SUFFIX, strlen(XATTR_SMACK_SUFFIX)))
+	if (strncmp(name, XATTR_NAME_SMACK, sizeof(XATTR_NAME_SMACK) - 1))
 		return 0;
 
 	if (strcmp(name, XATTR_NAME_SMACK) == 0 ||
-- 
2.43.0

[PATCH 02/19] smack: fix bug: changing Smack xattrs requires cap_mac_override

[Konstantin Andreev]

The xattr-changing hooks (smack_inode_setxattr,
smack_inode_removexattr) treat Smack xattrs
(security.SMACK64*) like any other xattrs,
but additionally require the subject process
to possess cap_mac_admin.

These hooks treat "any other xattr" as if it is
just labeled data, so changing a Smack xattr,
like changing any other xattr,
requires the subject process to either possess
cap_mac_override or have a rule allowing it
to write to the object.

A curious effect arises here: the rule 'foo bar w'
allows subject 'foo' to relabel object 'bar'
to any label 8-O - clearly not what the rules
are designed for.

According to the Smack documentation [1,2],
possessing cap_mac_admin is necessary and sufficient
to change Smack xattrs (security.SMACK64*).

Treating Smack xattrs as labeled data
appears to be incorrect.

This change excludes the "labeled data check" for
Smack xattrs from the aforementioned hooks, making
cap_mac_admin sufficient to change Smack xattrs.

(2008-02-04 Casey Schaufler)
Fixes: e114e473771c ("Smack: Simplified Mandatory Access Control Kernel")

[1] Documentation/admin-guide/LSM/Smack.rst

[2] 2012-06-05 Casey Schaufler
commit 1880eff77e7a ("Smack: onlycap limits on CAP_MAC_ADMIN")

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 105 +++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 51 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 42fdac05d32d..5a159a2653a6 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -175,7 +175,7 @@ static int smk_bu_task(struct task_struct *otp, int mode, int rc)
 #endif
 
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
-static int smk_bu_inode(struct inode *inode, int mode, int rc)
+static int smk_bu_inode(const struct inode * const inode, int mode, int rc)
 {
 	struct task_smack *tsp = smack_cred(current_cred());
 	struct inode_smack *isp = smack_inode(inode);
@@ -1343,65 +1343,67 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 				struct dentry *dentry, const char *name,
 				const void *value, size_t size, int flags)
 {
-	struct smk_audit_info ad;
-	struct smack_known *skp;
-	int check_priv = 0;
 	int check_import = 0;
 	int check_star = 0;
-	int rc = 0;
-	umode_t const i_mode = d_backing_inode(dentry)->i_mode;
+	struct inode * const inode = d_backing_inode(dentry);
+	umode_t const i_mode = inode->i_mode;
 
 	/*
 	 * Check label validity here so import won't fail in post_setxattr
 	 */
 	if (strcmp(name, XATTR_NAME_SMACK) == 0) {
 		/*
-		 * UDS inode has fixed label
+		 * inode of socket file descriptor (sockfs inode) and
+		 * UDS inode have fixed label
 		 */
-		if (S_ISSOCK(i_mode)) {
-			rc = -EINVAL;
-		} else {
-			check_priv = 1;
-			check_import = 1;
-		}
+		if (S_ISSOCK(i_mode))
+			return -EINVAL;
+		check_import = 1;
 	} else if (strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
-		check_priv = 1;
 		check_import = 1;
 	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
-		check_priv = 1;
 		check_import = 1;
 		check_star = 1;
 	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
-		check_priv = 1;
 		if (!S_ISDIR(i_mode) ||
 		    size != TRANS_TRUE_SIZE ||
 		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
-			rc = -EINVAL;
+			return -EINVAL;
+	} else {
+		/*
+		 * treat other xattrs as labeled data
+		 */
+		struct smk_audit_info ad;
+
+		smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
+		smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
+
+		return smk_bu_inode(inode, MAY_WRITE,
+		       smk_curacc(smk_of_inode(inode), MAY_WRITE, &ad));
 	}
 
-	if (check_priv && !smack_privileged(CAP_MAC_ADMIN))
-		rc = -EPERM;
+	if (!smack_privileged(CAP_MAC_ADMIN))
+		return -EPERM;
 
-	if (rc == 0 && check_import) {
-		skp = size ? smk_import_entry(value, size) : NULL;
+	if (check_import) {
+		const struct smack_known *skp;
+
+		if (size == 0)
+			return -EINVAL;
+
+		skp = smk_import_entry(value, size);
 		if (IS_ERR(skp))
-			rc = PTR_ERR(skp);
-		else if (skp == NULL || (check_star &&
-		    (skp == &smack_known_star || skp == &smack_known_web)))
-			rc = -EINVAL;
+			return PTR_ERR(skp);
+
+		if (check_star &&
+		    (skp == &smack_known_star ||
+		     skp == &smack_known_web))
+			return -EINVAL;
 	}
 
-	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
-	smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
-
-	if (rc == 0) {
-		rc = smk_curacc(smk_of_inode(d_backing_inode(dentry)), MAY_WRITE, &ad);
-		rc = smk_bu_inode(d_backing_inode(dentry), MAY_WRITE, rc);
-	}
-
-	return rc;
+	return 0;
 }
 
 /**
@@ -1452,6 +1454,9 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name,
  */
 static int smack_inode_getxattr(struct dentry *dentry, const char *name)
 {
+	/*
+	 * treat all xattrs as labeled data
+	 */
 	struct smk_audit_info ad;
 	int rc;
 
@@ -1476,9 +1481,8 @@ static int smack_inode_getxattr(struct dentry *dentry, const char *name)
 static int smack_inode_removexattr(struct mnt_idmap *idmap,
 				   struct dentry *dentry, const char *name)
 {
-	struct inode_smack *isp;
-	struct smk_audit_info ad;
-	int rc = 0;
+	const struct inode * const inode = d_backing_inode(dentry);
+	struct inode_smack * const isp = smack_inode(inode);
 
 	if (strcmp(name, XATTR_NAME_SMACK) == 0 ||
 	    strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
@@ -1487,21 +1491,20 @@ static int smack_inode_removexattr(struct mnt_idmap *idmap,
 	    strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0 ||
 	    strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
 		if (!smack_privileged(CAP_MAC_ADMIN))
-			rc = -EPERM;
+			return -EPERM;
+	} else {
+		/*
+		 * treat other xattrs as labeled data
+		 */
+		struct smk_audit_info ad;
+
+		smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
+		smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
+
+		return smk_bu_inode(inode, MAY_WRITE,
+		       smk_curacc(isp->smk_inode, MAY_WRITE, &ad));
 	}
 
-	if (rc != 0)
-		return rc;
-
-	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
-	smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
-
-	rc = smk_curacc(smk_of_inode(d_backing_inode(dentry)), MAY_WRITE, &ad);
-	rc = smk_bu_inode(d_backing_inode(dentry), MAY_WRITE, rc);
-	if (rc != 0)
-		return rc;
-
-	isp = smack_inode(d_backing_inode(dentry));
 	/*
 	 * Don't do anything special for these.
 	 *	XATTR_NAME_SMACKIPIN
@@ -1509,7 +1512,7 @@ static int smack_inode_removexattr(struct mnt_idmap *idmap,
 	 *	XATTR_NAME_SMACK if S_ISSOCK (UDS inode has fixed label)
 	 */
 	if (strcmp(name, XATTR_NAME_SMACK) == 0) {
-		if (!S_ISSOCK(d_backing_inode(dentry)->i_mode)) {
+		if (!S_ISSOCK(inode->i_mode)) {
 			struct super_block *sbp = dentry->d_sb;
 			struct superblock_smack *sbsp = smack_superblock(sbp);
 
-- 
2.43.0

[PATCH 03/19] smack: fix bug: setting label-containing xattrs silently ignores input garbage

[Konstantin Andreev]

The following command:
    # setfattr -n security.SMACK64 -v foo/bar FILE

sets the label of FILE to 'foo' w/o indication
that label does not match input.

This happens due to the use of an unsuitable parsing
function: smk_import_entry(), which acquires only
that part from the beginning of the input
that looks like a label.

This is exactly the same issue as described in [1],
but it occurs with Smack label-containing xattrs
instead of /proc/PID/attr/smack/current.

Curiously,

    # setfattr -n security.SMACK64     -v foo/bar FILE
    # setfattr -n security.SMACK64EXEC -v fo2/ba2 FILE
    # setfattr -n security.SMACK64MMAP -v fo3/ba3 FILE
    # getfattr -hd -m - FILE
    security.SMACK64="foo"            // garbage ignored
    security.SMACK64EXEC="fo2/ba2"    // garbage taken?!
    security.SMACK64MMAP="fo3/ba3"    // garbage taken?!

It looks like label-containing xattrs SMACK64EXEC
and SMACK64MMAP can acquire invalid Smack label.

In fact, inode contains the labels `fo2' and `fo3',
but, due to another Smack bug [2] we do not see them -
instead we see the on-disk content of the SMACK64EXEC
and SMACK64MMAP xattrs, that is, indeed, `fo2/ba2'
and `fo3/ba3'

This change detects input garbage by adding a check:
   taken label length == input length
and indicates -EINVAL to the caller if they do not match

(2008-02-04 Casey Schaufler)
Fixes: e114e473771c ("Smack: Simplified Mandatory Access Control Kernel")

[1] 2025-06-17 andreev
commit 674e2b24791c ("smack: fix bug: setting task label
                      silently ignores input garbage")
Link: https://lore.kernel.org/linux-security-module/20250315015723.1357541-3-andreev@swemel.ru/

[2] 2025-07 andreev (forward reference)
commit ("smack: fix bug: getxattr() returns invalid SMACK64EXEC/MMAP")
Link: https://lore.kernel.org/linux-security-module/ba2af0356940d61d932af15d4b1a265a61e7d16c.1753356770.git.andreev@swemel.ru/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 81 +++++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 24 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5a159a2653a6..4ef6355c84c0 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -263,6 +263,49 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
 #define smk_bu_credfile(cred, file, mode, RC) (RC)
 #endif
 
+/**
+ * smk_parse_label - check @string (not \0-terminated) is a valid Smack label
+ * @string: a string to check, not \0-terminated
+ * @string_len: the length of the @string
+ *
+ * Return: 0 or an error code
+ */
+static int
+smk_parse_label(const char *string, size_t string_len)
+{
+	int label_len;
+
+	if (unlikely(string == NULL || string_len == 0 ||
+		     string_len >= SMK_LONGLABEL))
+		return -EINVAL;
+
+	label_len = smk_parse_label_len(string, string_len);
+	if (label_len < 0 ||         /* invalid label */
+	    label_len != string_len) /* garbage after label */
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * smk_import_label - import a label, return the list entry
+ * @string: a text string that may be a valid Smack label, not \0-terminated
+ * @string_len: the length of the text string in the @string
+ *
+ * Contrast to smk_import_entry(), the full @string_len of the @string
+ * must constitute a valid Smack label to be imported.
+ *
+ * Return: see description of smk_import_entry()
+ */
+static struct smack_known *
+smk_import_label(const char *string, int string_len)
+{
+	if (smk_parse_label(string, string_len))
+		return ERR_PTR(-EINVAL);
+
+	return smk_import_valid_label(string, string_len, GFP_NOFS);
+}
+
 /**
  * smk_fetch - Fetch the smack label from a file.
  * @name: type of the label (attribute)
@@ -1343,14 +1386,11 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 				struct dentry *dentry, const char *name,
 				const void *value, size_t size, int flags)
 {
-	int check_import = 0;
+	bool label_inside = true;
 	int check_star = 0;
 	struct inode * const inode = d_backing_inode(dentry);
 	umode_t const i_mode = inode->i_mode;
 
-	/*
-	 * Check label validity here so import won't fail in post_setxattr
-	 */
 	if (strcmp(name, XATTR_NAME_SMACK) == 0) {
 		/*
 		 * inode of socket file descriptor (sockfs inode) and
@@ -1358,19 +1398,18 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 		 */
 		if (S_ISSOCK(i_mode))
 			return -EINVAL;
-		check_import = 1;
 	} else if (strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
-		check_import = 1;
+		;
 	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
-		check_import = 1;
 		check_star = 1;
 	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
 		if (!S_ISDIR(i_mode) ||
 		    size != TRANS_TRUE_SIZE ||
 		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
 			return -EINVAL;
+		label_inside = false;
 	} else {
 		/*
 		 * treat other xattrs as labeled data
@@ -1387,13 +1426,13 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 	if (!smack_privileged(CAP_MAC_ADMIN))
 		return -EPERM;
 
-	if (check_import) {
-		const struct smack_known *skp;
+	/*
+	 * Import label now, so import won't fail in smack_inode_post_setxattr
+	 */
+	if (label_inside) {
+		const struct smack_known * const skp =
+			smk_import_label(value, size);
 
-		if (size == 0)
-			return -EINVAL;
-
-		skp = smk_import_entry(value, size);
 		if (IS_ERR(skp))
 			return PTR_ERR(skp);
 
@@ -3781,23 +3820,17 @@ static int do_setattr(unsigned int attr, void *value, size_t size)
 	struct task_smack *tsp = smack_cred(current_cred());
 	struct cred *new;
 	struct smack_known *skp;
-	int label_len;
 
 	/*
 	 * let unprivileged user validate input, check permissions later
 	 */
-	if (value == NULL || size == 0 || size >= SMK_LONGLABEL)
+	if (smk_parse_label(value, size))
 		return -EINVAL;
-
-	label_len = smk_parse_label_len(value, size);
-	if (label_len < 0 || label_len != size)
-		return -EINVAL;
-
 	/*
 	 * No process is ever allowed the web ("@") label
 	 * and the star ("*") label.
 	 */
-	if (label_len == 1 /* '@', '*' */) {
+	if (size == 1 /* '@', '*' */) {
 		const char c = *(const char *)value;
 
 		if (c == *smack_known_web.smk_known ||
@@ -3810,8 +3843,8 @@ static int do_setattr(unsigned int attr, void *value, size_t size)
 		list_for_each_entry(sklep, &tsp->smk_relabel, list) {
 			const char *cp = sklep->smk_label->smk_known;
 
-			if (strlen(cp) == label_len &&
-			    strncmp(cp, value, label_len) == 0)
+			if (strlen(cp) == size &&
+			    strncmp(cp, value, size) == 0)
 				goto in_relabel;
 		}
 		return -EPERM;
@@ -3819,7 +3852,7 @@ static int do_setattr(unsigned int attr, void *value, size_t size)
 		;
 	}
 
-	skp = smk_import_valid_label(value, label_len, GFP_KERNEL);
+	skp = smk_import_valid_label(value, size, GFP_KERNEL);
 	if (IS_ERR(skp))
 		return PTR_ERR(skp);
 
-- 
2.43.0

[PATCH 04/19] smack: stop polling other LSMs & VFS to getxattr() unsupported SMACK64IPIN/OUT

[Konstantin Andreev]

smack_inode_getsecurity() returns -EOPNOTSUPP
for the SMACK64IPIN and SMACK64IPOUT xattrs
if the inode is not a socket.

Since [1], the -EOPNOTSUPP return value
from the inode_getsecurity LSM hook
has been interpreted by security_inode_getsecurity()
as a signal to "continue polling other LSMs".

As a result, security_inode_getsecurity() queries
other LSMs for these Smack xattrs.

Furthermore, the VFS layer is also aware of the convention
and attempts to read SMACK64IPIN and SMACK64IPOUT from disk
via __vfs_getxattr() if all LSMs return -EOPNOTSUPP.

Looking for Smack propertу in these places is incorrect,
as Smack does own these xattrs - even if
they are irrelevant for a particular inode.

Returning -ENODATA (no such attribute) instead of
-EOPNOTSUPP is more appropriate, as it stops further
fallback to other LSMs and the filesystem.

This appears safe, since __vfs_getxattr() also returns
-ENODATA when the attribute does not exist.

[1] 2016-05-31 Casey Schaufler
commit 2885c1e3e0c2 ("LSM: Fix for security_inode_getsecurity
                      and -EOPNOTSUPP")
Link: https://lore.kernel.org/lkml/d8a4d26e-46c8-975d-d075-a3848130981c@schaufler-ca.com/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4ef6355c84c0..7bd47baac481 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1649,10 +1649,6 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 				   struct inode *inode, const char *name,
 				   void **buffer, bool alloc)
 {
-	struct socket_smack *ssp;
-	struct socket *sock;
-	struct super_block *sbp;
-	struct inode *ip = inode;
 	struct smack_known *isp;
 	struct inode_smack *ispp;
 	size_t label_len;
@@ -1666,27 +1662,29 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 			label = TRANS_TRUE;
 		else
 			label = "";
-	} else {
+	} else if (strcmp(name, XATTR_SMACK_IPIN) == 0 ||
+		   strcmp(name, XATTR_SMACK_IPOUT) == 0) {
 		/*
-		 * The rest of the Smack xattrs are only on sockets.
+		 * These Smack xattrs are only on sockets.
 		 */
-		sbp = ip->i_sb;
-		if (sbp->s_magic != SOCKFS_MAGIC)
-			return -EOPNOTSUPP;
+		const struct socket_smack *ssp;
+		const struct sock *sk;
 
-		sock = SOCKET_I(ip);
-		if (sock == NULL || sock->sk == NULL)
-			return -EOPNOTSUPP;
+		if (inode->i_sb->s_magic != SOCKFS_MAGIC)
+			return -ENODATA;
 
-		ssp = smack_sock(sock->sk);
+		sk = SOCKET_I(inode)->sk;
+		if (sk == NULL)
+			return -ENODATA;
+
+		ssp = smack_sock(sk);
 
 		if (strcmp(name, XATTR_SMACK_IPIN) == 0)
 			isp = ssp->smk_in;
-		else if (strcmp(name, XATTR_SMACK_IPOUT) == 0)
-			isp = ssp->smk_out;
 		else
-			return -EOPNOTSUPP;
-	}
+			isp = ssp->smk_out;
+	} else
+		return -EOPNOTSUPP;
 
 	if (!label)
 		label = isp->smk_known;
-- 
2.43.0

[PATCH 05/19] smack: restrict getxattr() SMACK64TRANSMUTE to directories

[Konstantin Andreev]

Since [1], every filesystem object in the system
has a security.SMACK64TRANSMUTE xattr attached:
regular files, FIFOs, sockets, device nodes, and directories.

  # getfattr -n security.SMACK64TRANSMUTE /etc/passwd
  security.SMACK64TRANSMUTE=""
  # getfattr -n security.SMACK64TRANSMUTE /run/initctl
  security.SMACK64TRANSMUTE=""
  # getfattr -n security.SMACK64TRANSMUTE /run/udev/control
  security.SMACK64TRANSMUTE=""
  # getfattr -n security.SMACK64TRANSMUTE /dev/null
  security.SMACK64TRANSMUTE=""
  # getfattr -n security.SMACK64TRANSMUTE /etc
  security.SMACK64TRANSMUTE=""

Most of these values are blank (""), because the xattr
is not set for most directories, and is irrelevant for
anything else. Additionally,
blank is not a valid value for SMACK64TRANSMUTE.

Having an irrelevant xattr with an invalid value
on every filesystem object seems odd.

It is more appropriate to return -ENODATA
(no such attribute) for SMACK64TRANSMUTE,
if it is not set or irrelevant,
like for any other non-existent attribute.

[1] 2023-05-08 roberto.sassu
commit 3a3d8fce31a4 ("smack: Retrieve transmuting information in
                      smack_inode_getsecurity()")
Link: https://lore.kernel.org/linux-security-module/20230508170234.3595105-1-roberto.sassu@huaweicloud.com/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 7bd47baac481..7a27c554ac56 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1661,7 +1661,7 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 		if (ispp->smk_flags & SMK_INODE_TRANSMUTE)
 			label = TRANS_TRUE;
 		else
-			label = "";
+			return -ENODATA;
 	} else if (strcmp(name, XATTR_SMACK_IPIN) == 0 ||
 		   strcmp(name, XATTR_SMACK_IPOUT) == 0) {
 		/*
-- 
2.43.0

[PATCH 06/19] smack: fix bug: getxattr() returns invalid SMACK64EXEC/MMAP

[Konstantin Andreev]

The Smack inode_getsecurity hook, smack_inode_getsecurity(),
does not provide the values of the SMACK64EXEC and SMACK64MMAP
xattrs from the inode. Instead, it returns -EOPNOTSUPP
to signal that these xattrs are not handled.

Since [1], the -EOPNOTSUPP return value
from the inode_getsecurity LSM hook
has been interpreted by security_inode_getsecurity()
as a signal to "continue polling other LSMs".

As a result, security_inode_getsecurity() continues querying
other LSMs and eventually returns control to vfs_getxattr(),
which attempts to read these xattrs from disk
via __vfs_getxattr()

There are two issues here:

1) querying a Smack property from other LSMs
   is incorrect, as Smack does own these xattrs.

2) the on-disk xattrs may differ from the in-memory
   labels actually associated with the inode.

The SMACK64EXEC/MMAP xattr on disk may contain an invalid
label (e.g. foo/bar, see [2]), a forbidden one (*, @),
or even corrupted data.

Such values are treated somehow during inode instantiation
and may produce a valid Smack label or no label at all,
but, anyway, one that differs from the on-disk value

This change ensures that getxattr() returns the actual
in-memory label associated with the inode,
not the raw xattr from disk.

[1] 2016-05-31 Casey Schaufler
commit 2885c1e3e0c2 ("LSM: Fix for security_inode_getsecurity
                      and -EOPNOTSUPP")
Link: https://lore.kernel.org/lkml/d8a4d26e-46c8-975d-d075-a3848130981c@schaufler-ca.com/

[2] 2025-07 andreev
commit ("smack: fix bug: setting label-containing xattrs
         silently ignores input garbage")
Link: https://lore.kernel.org/linux-security-module/ae1100894499a1f6ce8e783727635388b3ac3af8.1753356770.git.andreev@swemel.ru/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 7a27c554ac56..052404e2fda6 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1649,17 +1649,20 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 				   struct inode *inode, const char *name,
 				   void **buffer, bool alloc)
 {
-	struct smack_known *isp;
-	struct inode_smack *ispp;
-	size_t label_len;
-	char *label = NULL;
+	const struct smack_known *skp;
+	const struct inode_smack * const isp = smack_inode(inode);
+	const char *value = NULL;
+	int value_len;
 
 	if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
-		isp = smk_of_inode(inode);
+		skp = isp->smk_inode;
+	} else if (strcmp(name, XATTR_SMACK_EXEC) == 0) {
+		skp = isp->smk_task;
+	} else if (strcmp(name, XATTR_SMACK_MMAP) == 0) {
+		skp = isp->smk_mmap;
 	} else if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) {
-		ispp = smack_inode(inode);
-		if (ispp->smk_flags & SMK_INODE_TRANSMUTE)
-			label = TRANS_TRUE;
+		if (isp->smk_flags & SMK_INODE_TRANSMUTE)
+			value = TRANS_TRUE;
 		else
 			return -ENODATA;
 	} else if (strcmp(name, XATTR_SMACK_IPIN) == 0 ||
@@ -1680,24 +1683,27 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 		ssp = smack_sock(sk);
 
 		if (strcmp(name, XATTR_SMACK_IPIN) == 0)
-			isp = ssp->smk_in;
+			skp = ssp->smk_in;
 		else
-			isp = ssp->smk_out;
+			skp = ssp->smk_out;
 	} else
 		return -EOPNOTSUPP;
 
-	if (!label)
-		label = isp->smk_known;
+	if (!value) {
+		if (!skp)
+			return -ENODATA;
+		value = skp->smk_known;
+	}
 
-	label_len = strlen(label);
+	value_len = strlen(value);
 
 	if (alloc) {
-		*buffer = kstrdup(label, GFP_KERNEL);
+		*buffer = kmemdup(value, value_len, GFP_KERNEL);
 		if (*buffer == NULL)
 			return -ENOMEM;
 	}
 
-	return label_len;
+	return value_len;
 }
 
 
-- 
2.43.0

[PATCH 07/19] smack: deduplicate task label validation

[Konstantin Andreev]

The part of the description of smk_task_invalid_label()
("Smack prohibits ... explicitly.")
was taken verbatim from the commit message of:

2013-12-16 Casey Schaufler
commit 19760ad03cc6 ("Smack: Prevent the * and @ labels
                      from being used in SMACK64EXEC")

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 47 +++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 052404e2fda6..39e2e7b5bc3c 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1369,6 +1369,21 @@ static int smack_inode_xattr_skipcap(const char *name)
 	return 0;
 }
 
+/*
+ * Smack prohibits processes from using the star ("*") and web ("@") labels
+ * because we don't want files with those labels getting created implicitly.
+ * All setting of those labels should be done explicitly.
+ *
+ * Comparing smack_known's assumes label has been imported. Until only builtin
+ * labels are prohibited, just imported label is not eligible for rejection.
+ */
+static bool
+smk_task_invalid_label(const struct smack_known * const skp)
+{
+	return skp == &smack_known_web ||
+	       skp == &smack_known_star;
+}
+
 /**
  * smack_inode_setxattr - Smack check for setting xattrs
  * @idmap: idmap of the mount
@@ -1387,7 +1402,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 				const void *value, size_t size, int flags)
 {
 	bool label_inside = true;
-	int check_star = 0;
+	bool task_label = false;
 	struct inode * const inode = d_backing_inode(dentry);
 	umode_t const i_mode = inode->i_mode;
 
@@ -1403,7 +1418,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 		;
 	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
-		check_star = 1;
+		task_label = true;
 	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
 		if (!S_ISDIR(i_mode) ||
 		    size != TRANS_TRUE_SIZE ||
@@ -1436,10 +1451,10 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 		if (IS_ERR(skp))
 			return PTR_ERR(skp);
 
-		if (check_star &&
-		    (skp == &smack_known_star ||
-		     skp == &smack_known_web))
-			return -EINVAL;
+		if (task_label) {
+			if (smk_task_invalid_label(skp))
+				return -EINVAL;
+		}
 	}
 
 	return 0;
@@ -3726,14 +3741,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
 		 * Don't let the exec or mmap label be "*" or "@".
 		 */
 		skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
-		if (IS_ERR(skp) || skp == &smack_known_star ||
-		    skp == &smack_known_web)
+		if (IS_ERR(skp) || smk_task_invalid_label(skp))
 			skp = NULL;
 		isp->smk_task = skp;
 
 		skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
-		if (IS_ERR(skp) || skp == &smack_known_star ||
-		    skp == &smack_known_web)
+		if (IS_ERR(skp) || smk_task_invalid_label(skp))
 			skp = NULL;
 		isp->smk_mmap = skp;
 
@@ -3830,17 +3843,6 @@ static int do_setattr(unsigned int attr, void *value, size_t size)
 	 */
 	if (smk_parse_label(value, size))
 		return -EINVAL;
-	/*
-	 * No process is ever allowed the web ("@") label
-	 * and the star ("*") label.
-	 */
-	if (size == 1 /* '@', '*' */) {
-		const char c = *(const char *)value;
-
-		if (c == *smack_known_web.smk_known ||
-		    c == *smack_known_star.smk_known)
-			return -EPERM;
-	}
 
 	if (!smack_privileged(CAP_MAC_ADMIN)) {
 		const struct smack_known_list_elem *sklep;
@@ -3860,6 +3862,9 @@ static int do_setattr(unsigned int attr, void *value, size_t size)
 	if (IS_ERR(skp))
 		return PTR_ERR(skp);
 
+	if (smk_task_invalid_label(skp))
+		return -EPERM;
+
 	new = prepare_creds();
 	if (new == NULL)
 		return -ENOMEM;
-- 
2.43.0

[PATCH 08/19] smack: smack_inode_setsecurity: prevent setting SMACK64EXEC/MMAP in other LSMs

[Konstantin Andreev]

smack_inode_setsecurity() does not support setting
the SMACK64EXEC and SMACK64MMAP xattrs, and returns
-EOPNOTSUPP if an attempt is made to set them.

However, since [1], the -EOPNOTSUPP return value
from the inode_setsecurity LSM hook
has been interpreted by security_inode_setsecurity()
as a signal to "continue polling other LSMs".

As a result, security_inode_setsecurity() proceeds
to query other LSMs and attempts to store the
SMACK64EXEC/MMAP xattrs there.

Passing a Smack property to other LSMs is incorrect,
as Smack owns these xattrs.

Returning -ENODATA instead of -EOPNOTSUPP is not ideal,
but it makes sense and prevents fallback to other LSMs.

[1] 2016-05-31 Casey Schaufler
commit 2885c1e3e0c2 ("LSM: Fix for security_inode_getsecurity
                      and -EOPNOTSUPP")
Link: https://lore.kernel.org/lkml/d8a4d26e-46c8-975d-d075-a3848130981c@schaufler-ca.com/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 39e2e7b5bc3c..00d4b5bf1056 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3000,6 +3000,10 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 		nsp->smk_flags |= SMK_INODE_INSTANT;
 		return 0;
 	}
+
+	if (strcmp(name, XATTR_SMACK_EXEC) == 0 ||
+	    strcmp(name, XATTR_SMACK_MMAP) == 0)
+		return -ENODATA;
 	/*
 	 * The rest of the Smack xattrs are only on sockets.
 	 */
-- 
2.43.0

[PATCH 09/19] smack: smack_inode_setsecurity: prevent setting SMACK64IPIN/OUT in other LSMs

[Konstantin Andreev]

smack_inode_setsecurity() equally returns -EOPNOTSUPP
either the inode does not come from socket (sockfs
inode) or the xattr is a security.NOT-SMACK-XATTR

This did make no difference until [1]. Since [1]
-EOPNOTSUPP is reserved by security_inode_setsecurity()
as a signal to "continue polling other LSMs".

When xattr is SMACK64IPIN or SMACK64IPOUT and inode
is not from socket then return code is -EOPNOTSUPP,
and the security_inode_setsecurity() proceeds to query
other LSMs and attempts to store the xattr there.

Passing a Smack property to other LSMs is incorrect,
as Smack owns these xattrs.

This change returns -ENODATA if inode does not come
from sockfs and the xattr is SMACK64IPIN/OUT.
This causes change from

  # setfattr -n security.SMACK64IPIN -v foo /sys/kernel/debug/sleep_time
  setfattr: /sys/kernel/debug/sleep_time: Operation not supported

to

  # setfattr -n security.SMACK64IPIN -v foo /sys/kernel/debug/sleep_time
  setfattr: /sys/kernel/debug/sleep_time: No such attribute

not ideal, but it makes sense and prevents fallback to other LSMs.

[1] 2016-05-31 Casey Schaufler
commit 2885c1e3e0c2 ("LSM: Fix for security_inode_getsecurity
                      and -EOPNOTSUPP")
Link: https://lore.kernel.org/lkml/d8a4d26e-46c8-975d-d075-a3848130981c@schaufler-ca.com/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 00d4b5bf1056..7108696083d8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3004,21 +3004,25 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	if (strcmp(name, XATTR_SMACK_EXEC) == 0 ||
 	    strcmp(name, XATTR_SMACK_MMAP) == 0)
 		return -ENODATA;
+
+	if (!(strcmp(name, XATTR_SMACK_IPIN) == 0 ||
+	      strcmp(name, XATTR_SMACK_IPOUT) == 0))
+		return -EOPNOTSUPP;
 	/*
 	 * The rest of the Smack xattrs are only on sockets.
 	 */
 	if (inode->i_sb->s_magic != SOCKFS_MAGIC)
-		return -EOPNOTSUPP;
+		return -ENODATA;
 
 	sock = SOCKET_I(inode);
-	if (sock == NULL || sock->sk == NULL)
-		return -EOPNOTSUPP;
+	if (sock->sk == NULL)
+		return -ENODATA;
 
 	ssp = smack_sock(sock->sk);
 
 	if (strcmp(name, XATTR_SMACK_IPIN) == 0)
 		ssp->smk_in = skp;
-	else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) {
+	else {
 		ssp->smk_out = skp;
 		if (sock->sk->sk_family == PF_INET) {
 			rc = smack_netlbl_add(sock->sk);
@@ -3027,8 +3031,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 					"Smack: \"%s\" netlbl error %d.\n",
 					__func__, -rc);
 		}
-	} else
-		return -EOPNOTSUPP;
+	}
 
 #ifdef SMACK_IPV6_PORT_LABELING
 	if (sock->sk->sk_family == PF_INET6)
-- 
2.43.0

[PATCH 10/19] smack: fix bug: smack_inode_setsecurity() imports alien xattrs as labels

[Konstantin Andreev]

Currently, smack_inode_setsecurity() calls smk_import_entry()
to import the xattr value as a label before checking whether
the xattr is actually a Smack xattr.

For example, attempting to set security.foo=bar on a socket
fails as expected, but the value 'bar' is still imported
into Smack as a label.

This change ensures that the xattr is recognized
as a Smack xattr before importing its value.

2008-02-04 Casey Schaufler
Fixes: e114e473771c ("Smack: Simplified Mandatory Access Control Kernel")

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 7108696083d8..6f74be82ae45 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2991,6 +2991,14 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 		return 0;
 	}
 
+	if (!(strcmp(name, XATTR_SMACK_SUFFIX) == 0 ||
+	      strcmp(name, XATTR_SMACK_EXEC) == 0 ||
+	      strcmp(name, XATTR_SMACK_MMAP) == 0 ||
+	      strcmp(name, XATTR_SMACK_IPIN) == 0 ||
+	      strcmp(name, XATTR_SMACK_IPOUT) == 0
+	))
+		return -EOPNOTSUPP;
+
 	skp = smk_import_entry(value, size);
 	if (IS_ERR(skp))
 		return PTR_ERR(skp);
@@ -3004,10 +3012,6 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	if (strcmp(name, XATTR_SMACK_EXEC) == 0 ||
 	    strcmp(name, XATTR_SMACK_MMAP) == 0)
 		return -ENODATA;
-
-	if (!(strcmp(name, XATTR_SMACK_IPIN) == 0 ||
-	      strcmp(name, XATTR_SMACK_IPOUT) == 0))
-		return -EOPNOTSUPP;
 	/*
 	 * The rest of the Smack xattrs are only on sockets.
 	 */
-- 
2.43.0

[PATCH 11/19] smack: fix bug: smack_inode_setsecurity() false EINVAL for alien xattrs

[Konstantin Andreev]

Currently, smack_inode_setsecurity() checks
the validity of the xattr value before checking
whether the xattr is actually a Smack xattr.

This was correct when only one LSM
could be active in the system.
Since [1] this is no longer incorrect.

When Smack mistakenly EINVALidates a non-Smack xattr,
Smack may prevent owner LSM from seeing the xattr.

The change ensures that the xattr is recognized
as a Smack xattr before looking into the value.

[1] 2015-05-02 Casey Schaufler
Fixes: b1d9e6b0646d ("LSM: Switch to lists of hooks")

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 6f74be82ae45..672be8b47821 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2979,6 +2979,15 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	struct socket *sock;
 	int rc = 0;
 
+	if (!(strcmp(name, XATTR_SMACK_SUFFIX) == 0 ||
+	      strcmp(name, XATTR_SMACK_TRANSMUTE) == 0 ||
+	      strcmp(name, XATTR_SMACK_EXEC) == 0 ||
+	      strcmp(name, XATTR_SMACK_MMAP) == 0 ||
+	      strcmp(name, XATTR_SMACK_IPIN) == 0 ||
+	      strcmp(name, XATTR_SMACK_IPOUT) == 0
+	))
+		return -EOPNOTSUPP;
+
 	if (value == NULL || size > SMK_LONGLABEL || size == 0)
 		return -EINVAL;
 
@@ -2991,14 +3000,6 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 		return 0;
 	}
 
-	if (!(strcmp(name, XATTR_SMACK_SUFFIX) == 0 ||
-	      strcmp(name, XATTR_SMACK_EXEC) == 0 ||
-	      strcmp(name, XATTR_SMACK_MMAP) == 0 ||
-	      strcmp(name, XATTR_SMACK_IPIN) == 0 ||
-	      strcmp(name, XATTR_SMACK_IPOUT) == 0
-	))
-		return -EOPNOTSUPP;
-
 	skp = smk_import_entry(value, size);
 	if (IS_ERR(skp))
 		return PTR_ERR(skp);
-- 
2.43.0

[PATCH 12/19] smack: restrict setxattr() SMACK64IPIN/IPOUT to sockets

[Konstantin Andreev]

The SMACK64IPIN and SMACK64IPOUT xattrs apply
only to sockets. However, smack_inode_setxattr()
currently allows setting them on any filesystem
object, including regular files, FIFOs, and others.

These xattrs are even written to disk by the
underlying filesystem. E.g. you can

  # setfattr -n security.SMACK64IPIN -v foo /etc/passwd
  # # no error

and have SMACK64IPIN on disk.

This change restricts setting SMACK64IPIN/IPOUT
in smack_inode_setxattr() to socket inodes only.

Given that, the corresponding check in
smack_inode_setsecurity() may be omitted,
as it called after smack_inode_setxattr()
for SMACK64IPIN/IPOUT:

    fs/xattr.c:

    ...
    ` __vfs_setxattr_locked
      ` security_inode_setxattr
      ` __vfs_setxattr_noperm
        ` security_inode_setsecurity

Additionally, with this change the error code returned by setxattr()
for unsupported SMACK64IPIN/OUT xattrs
changes from -ENODATA [1]:

  # setfattr -n security.SMACK64IPIN -v foo /sys/kernel/debug/sleep_time
  setfattr: /sys/kernel/debug/sleep_time: No such attribute

back to -EOPNOTSUPP:

  # setfattr -n security.SMACK64IPIN -v foo /sys/kernel/debug/sleep_time
  setfattr: /sys/kernel/debug/sleep_time: Operation not supported

[1] 2025-07 andreev
commit ("smack: smack_inode_setsecurity:
         prevent setting SMACK64IPIN/OUT in other LSMs")
Link: https://lore.kernel.org/linux-security-module/a0d039a407a8164a2025847f5b00fd5f3c2e5def.1753356770.git.andreev@swemel.ru/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 Documentation/admin-guide/LSM/Smack.rst |  3 ++-
 security/smack/smack_lsm.c              | 16 +++++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/LSM/Smack.rst b/Documentation/admin-guide/LSM/Smack.rst
index c5ed775f2d10..ce8be25333a7 100644
--- a/Documentation/admin-guide/LSM/Smack.rst
+++ b/Documentation/admin-guide/LSM/Smack.rst
@@ -693,7 +693,8 @@ can only be set by privileged tasks, but any task can read them for their own
 sockets.
 
   SMACK64IPIN:
-	The Smack label of the task object. A privileged
+	The Smack label of incoming packets must have write access to the
+	Smack label, specified in the SMACK64IPIN attribute. A privileged
 	program that will enforce policy may set this to the star label.
 
   SMACK64IPOUT:
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 672be8b47821..a66fa2c16dc2 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1415,7 +1415,14 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 			return -EINVAL;
 	} else if (strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
-		;
+		/*
+		 * inode of socket file descriptor (sockfs inode) only
+		 */
+		if (inode->i_sb->s_magic != SOCKFS_MAGIC)
+			return -EOPNOTSUPP;
+
+		if (SOCKET_I(inode)->sk == NULL)
+			return -EOPNOTSUPP;
 	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
 		task_label = true;
@@ -3015,14 +3022,9 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 		return -ENODATA;
 	/*
 	 * The rest of the Smack xattrs are only on sockets.
+	 * smack_inode_setxattr() has checked that inode is sockfs
 	 */
-	if (inode->i_sb->s_magic != SOCKFS_MAGIC)
-		return -ENODATA;
-
 	sock = SOCKET_I(inode);
-	if (sock->sk == NULL)
-		return -ENODATA;
-
 	ssp = smack_sock(sock->sk);
 
 	if (strcmp(name, XATTR_SMACK_IPIN) == 0)
-- 
2.43.0

[PATCH 13/19] smack: restrict setxattr() SMACK64EXEC/MMAP to regular files

[Konstantin Andreev]

The SMACK64EXEC and SMACK64MMAP xattrs apply
only to regular files. However, setxattr() currently
allows setting them on any filesystem object,
including FIFOs, device nodes, and others. E.g.

   root# setfattr  -n security.SMACK64EXEC -v foo /dev/null
   root# getfattr -hn security.SMACK64EXEC        /dev/null
   # file: dev/null
   security.SMACK64EXEC="foo"

This change restricts setting SMACK64EXEC and
SMACK64MMAP to regular files only.

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a66fa2c16dc2..6712fa047722 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1425,6 +1425,8 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 			return -EOPNOTSUPP;
 	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
+		if (!S_ISREG(i_mode))
+			return -EOPNOTSUPP;
 		task_label = true;
 	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
 		if (!S_ISDIR(i_mode) ||
@@ -3754,15 +3756,17 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
 		/*
 		 * Don't let the exec or mmap label be "*" or "@".
 		 */
-		skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
-		if (IS_ERR(skp) || smk_task_invalid_label(skp))
-			skp = NULL;
-		isp->smk_task = skp;
+		if (S_ISREG(inode->i_mode)) {
+			skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
+			if (IS_ERR(skp) || smk_task_invalid_label(skp))
+				skp = NULL;
+			isp->smk_task = skp;
 
-		skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
-		if (IS_ERR(skp) || smk_task_invalid_label(skp))
-			skp = NULL;
-		isp->smk_mmap = skp;
+			skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
+			if (IS_ERR(skp) || smk_task_invalid_label(skp))
+				skp = NULL;
+			isp->smk_mmap = skp;
+		}
 
 		dput(dp);
 		break;
-- 
2.43.0

[PATCH 14/19] smack: return EOPNOTSUPP for setxattr() unsupported SMACK64(TRANSMUTE)

[Konstantin Andreev]

The SMACK64TRANSMUTE is supported only for directories.

"The standard return value for unsupported attribute names is
 -EOPNOTSUPP, as opposed to undefined but supported attributes
 (-ENODATA)" [3]

Smack follows [4] the convention
for get/setxattr() SMACK64IPIN/IPOUT

It is more appropriate to return -EOPNOTSUPP
instead of -EINVAL when attempting to set SMACK64TRANSMUTE
on a non-directory object.

A Unix domain socket (UDS) with a BSD address, and
the inode of a socket file descriptor (sockfs inode)
have fixed (*) nominal SMACK64 label [2]

Likewise, it is more appropriate to return -EOPNOTSUPP
instead of -EINVAL when attempting to set
the SMACK64 xattr on either type of socket inode.

The commits being fixed [1,2] are recent enough
so this change should not break userspace.

[1] 2023-11-16 roberto.sassu
Fixes: 9c82169208dd ("smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()")
Link: https://lore.kernel.org/linux-security-module/20231116090125.187209-2-roberto.sassu@huaweicloud.com/

[2] 2025-06-16 andreev
Fixes: 78fc6a94be25 ("smack: fix bug: invalid label of unix socket file")
Link: https://lore.kernel.org/linux-security-module/20250616010745.800386-6-andreev@swemel.ru/

[3] 2016-09-29 agruenba
commit 971df15bd54a ("sockfs: getxattr: Fail with -EOPNOTSUPP
                      for invalid attribute names")

[4] 2008-02-04 casey
commit e114e473771c ("Smack: Simplified Mandatory Access Control Kernel")

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 6712fa047722..113371887b4d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1412,7 +1412,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 		 * UDS inode have fixed label
 		 */
 		if (S_ISSOCK(i_mode))
-			return -EINVAL;
+			return -EOPNOTSUPP;
 	} else if (strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
 		   strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
 		/*
@@ -1429,8 +1429,9 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 			return -EOPNOTSUPP;
 		task_label = true;
 	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
-		if (!S_ISDIR(i_mode) ||
-		    size != TRANS_TRUE_SIZE ||
+		if (!S_ISDIR(i_mode))
+			return -EOPNOTSUPP;
+		if (size != TRANS_TRUE_SIZE ||
 		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
 			return -EINVAL;
 		label_inside = false;
-- 
2.43.0

[PATCH 15/19] smack: smack_inode_setsecurity(): skip checks for SMACK64TRANSMUTE

[Konstantin Andreev]

Like smack_inode_post_setxattr(), smack_inode_setsecurity()
is called after smack_inode_setxattr()
for the SMACK64TRANSMUTE xattr:

    fs/xattr.c:

    ...
    ` __vfs_setxattr_locked
      ` security_inode_setxattr
      ` __vfs_setxattr_noperm
        |
        ` security_inode_post_setxattr
        |    ^
        |     }- one of them is called
        |    v
        ` security_inode_setsecurity

Like smack_inode_post_setxattr(), smack_inode_setsecurity()
does not need to check
the applicability of the SMACK64TRANSMUTE or its value,
as they have already been checked by smack_inode_setxattr()

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 113371887b4d..5175dfb3d448 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3002,10 +3002,6 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 		return -EINVAL;
 
 	if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) {
-		if (!S_ISDIR(inode->i_mode) || size != TRANS_TRUE_SIZE ||
-		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
-			return -EINVAL;
-
 		nsp->smk_flags |= SMK_INODE_TRANSMUTE;
 		return 0;
 	}
-- 
2.43.0

[PATCH 16/19] smack: smack_inode_notifysecctx(): reject invalid labels

[Konstantin Andreev]

Exactly the same issue as described in [1,2].

    smack_inode_notifysecctx()
    ` smack_inode_setsecurity
      ` smk_import_entry

uses an unsuitable parsing
function: smk_import_entry(), which acquires only
that part from the beginning of the input
that looks like a label.

[1] 2025-06-17 andreev
commit 674e2b24791c ("smack: fix bug: setting task label
                      silently ignores input garbage")
Link: https://lore.kernel.org/linux-security-module/20250315015723.1357541-3-andreev@swemel.ru/

[2] 2025-07 andreev
commit ("smack: fix bug: setting label-containing xattrs
         silently ignores input garbage")
Link: https://lore.kernel.org/linux-security-module/ae1100894499a1f6ce8e783727635388b3ac3af8.1753356770.git.andreev@swemel.ru/

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5175dfb3d448..9271cd54bc43 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -5003,11 +5003,17 @@ static int smack_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
 
 static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
 {
+	const struct smack_known *skp;
 	/*
 	 * UDS inode has fixed label. Ignore nfs label.
 	 */
 	if (S_ISSOCK(inode->i_mode))
 		return 0;
+
+	skp = smk_import_label(ctx, ctxlen);
+	if (IS_ERR(skp))
+		return PTR_ERR(skp);
+
 	return smack_inode_setsecurity(inode, XATTR_SMACK_SUFFIX, ctx,
 				       ctxlen, 0);
 }
-- 
2.43.0

[PATCH 17/19] smack: smack_inode_post_setxattr(): find label instead of import

[Konstantin Andreev]

Efforts were made in [1]
to ensure that smk_import_entry() can not fail
when called by smack_inode_post_setxattr():
label is imported in advance by smack_inode_setxattr().

However,

  - smk_import_entry() can still fail
    due to memory allocation

  - its use is misleading here,
    as no actual import is needed

Using smk_find_entry() instead of smk_import_entry()
should be sufficient for smack_inode_post_setxattr().

However, smk_find_entry() takes a \0-terminated string,
while we have a (non-\0-terminated string, length) tuple.

To resolve this, I added smk_find_label_rcu(),
which accepts such a tuple and
is otherwise identical to smk_find_entry().
It is now used in smack_inode_post_setxattr().

smack_inode_post_setxattr() can no longer fail.

[1] 2009-04-16 etienne.basset
commit defc433ba3bc ("Smack: check for SMACK xattr validity
                      in smack_inode_setxattr")

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack.h        |  2 ++
 security/smack/smack_access.c | 22 ++++++++++++++++++++--
 security/smack/smack_lsm.c    | 26 ++++++++++++--------------
 3 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 759343a6bbae..9abb11947fe9 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -294,6 +294,8 @@ struct smack_known *smk_import_valid_label(const char *label, int label_len,
 					   gfp_t gfp);
 void smk_insert_entry(struct smack_known *skp);
 struct smack_known *smk_find_entry(const char *);
+struct smack_known *smk_find_label_rcu(const char *string, size_t string_len);
+struct smack_known *smk_find_label(const char *string, size_t string_len);
 bool smack_privileged(int cap);
 bool smack_privileged_cred(int cap, const struct cred *cred);
 void smk_destroy_label_list(struct list_head *list);
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 09167be79122..390dc9642f9b 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -427,21 +427,39 @@ void smk_insert_entry(struct smack_known *skp)
  * matches the passed string or NULL if not found.
  */
 struct smack_known *smk_find_entry(const char *string)
+{
+	return smk_find_label_rcu(string, strlen(string));
+}
+
+struct smack_known *
+smk_find_label_rcu(const char *string, size_t string_len)
 {
 	unsigned int hash;
 	struct hlist_head *head;
 	struct smack_known *skp;
 
-	hash = full_name_hash(NULL, string, strlen(string));
+	hash = full_name_hash(NULL, string, string_len);
 	head = &smack_known_hash[hash & (SMACK_HASH_SLOTS - 1)];
 
 	hlist_for_each_entry_rcu(skp, head, smk_hashed)
-		if (strcmp(skp->smk_known, string) == 0)
+		if (strlen(skp->smk_known) == string_len &&
+		   strncmp(skp->smk_known, string, string_len) == 0)
 			return skp;
 
 	return NULL;
 }
 
+struct smack_known *
+smk_find_label(const char *string, size_t string_len)
+{
+	struct smack_known *skp;
+
+	rcu_read_lock();
+	skp = smk_find_label_rcu(string, string_len);
+	rcu_read_unlock();
+	return skp;
+}
+
 /**
  * smk_parse_label_len - calculate the length of the starting segment
  *                       in the string that constitutes a valid smack label
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 9271cd54bc43..5d3d72162444 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1484,7 +1484,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 static void smack_inode_post_setxattr(struct dentry *dentry, const char *name,
 				      const void *value, size_t size, int flags)
 {
-	struct smack_known *skp;
+	struct smack_known **skpp = NULL;
 	struct inode_smack *isp = smack_inode(d_backing_inode(dentry));
 
 	if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
@@ -1492,19 +1492,17 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name,
 		return;
 	}
 
-	if (strcmp(name, XATTR_NAME_SMACK) == 0) {
-		skp = smk_import_entry(value, size);
-		if (!IS_ERR(skp))
-			isp->smk_inode = skp;
-	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0) {
-		skp = smk_import_entry(value, size);
-		if (!IS_ERR(skp))
-			isp->smk_task = skp;
-	} else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
-		skp = smk_import_entry(value, size);
-		if (!IS_ERR(skp))
-			isp->smk_mmap = skp;
-	}
+	if (strcmp(name, XATTR_NAME_SMACK) == 0)
+		skpp = &isp->smk_inode;
+	else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0)
+		skpp = &isp->smk_task;
+	else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0)
+		skpp = &isp->smk_mmap;
+	/*
+	 * Label has been imported by smack_inode_setxattr, just find it
+	 */
+	if (skpp)
+		*skpp = smk_find_label(value, size);
 
 	return;
 }
-- 
2.43.0

[PATCH 18/19] smack: smack_inode_setsecurity(): find label instead of import

[Konstantin Andreev]

smack_inode_setsecurity() is called
for label-containing xattrs either
after smack_inode_setxattr():

    fs/xattr.c:

    ...
    ` __vfs_setxattr_locked
      ` security_inode_setxattr
      ` __vfs_setxattr_noperm
        ` security_inode_setsecurity
or
    ...
    ` smack_inode_notifysecctx
      ` smack_inode_setsecurity

In both cases - via security_inode_setxattr()
or smack_inode_notifysecctx() -
the label is imported in advance.

There is no need to validate and import
the input value again; looking it up is sufficient.

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5d3d72162444..6c529de00584 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2996,17 +2996,16 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	))
 		return -EOPNOTSUPP;
 
-	if (value == NULL || size > SMK_LONGLABEL || size == 0)
-		return -EINVAL;
-
 	if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) {
 		nsp->smk_flags |= SMK_INODE_TRANSMUTE;
 		return 0;
 	}
 
-	skp = smk_import_entry(value, size);
-	if (IS_ERR(skp))
-		return PTR_ERR(skp);
+	if (strcmp(name, XATTR_SMACK_EXEC) == 0 ||
+	    strcmp(name, XATTR_SMACK_MMAP) == 0)
+		return -ENODATA;
+
+	skp = smk_find_label(value, size);
 
 	if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
 		nsp->smk_inode = skp;
@@ -3014,9 +3013,6 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 		return 0;
 	}
 
-	if (strcmp(name, XATTR_SMACK_EXEC) == 0 ||
-	    strcmp(name, XATTR_SMACK_MMAP) == 0)
-		return -ENODATA;
 	/*
 	 * The rest of the Smack xattrs are only on sockets.
 	 * smack_inode_setxattr() has checked that inode is sockfs
-- 
2.43.0

[PATCH 19/19] smack: deduplicate strcmp(name, XATTR_{,NAME_}SMACK*)

[Konstantin Andreev]

Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
---
 security/smack/smack_lsm.c | 170 ++++++++++++++++++++++++-------------
 1 file changed, 109 insertions(+), 61 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 6c529de00584..81c0f69202ea 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -96,6 +96,49 @@ static int match_opt_prefix(char *s, int l, char **arg)
 	return Opt_error;
 }
 
+enum smack_xa {
+	SMK_XA_OBJECT,    // security.SMACK64
+	SMK_XA_IPIN,      // security.SMACK64IPIN
+	SMK_XA_IPOUT,     // security.SMACK64IPOUT
+	SMK_XA_EXEC,      // security.SMACK64EXEC
+	SMK_XA_MMAP,      // security.SMACK64MMAP
+	SMK_XA_TRANSMUTE, // security.SMACK64TRANSMUTE
+};
+
+static const char * const
+smk_xa_suffix[] = {
+	[SMK_XA_OBJECT]    = "",
+	[SMK_XA_IPIN]      = "IPIN",
+	[SMK_XA_IPOUT]     = "IPOUT",
+	[SMK_XA_EXEC]      = "EXEC",
+	[SMK_XA_MMAP]      = "MMAP",
+	[SMK_XA_TRANSMUTE] = "TRANSMUTE",
+};
+
+static int
+smk_xa_suffix_to_id(const char *suffix)
+{
+	return match_string(smk_xa_suffix, ARRAY_SIZE(smk_xa_suffix), suffix);
+}
+
+static int
+smk_xa_name_to_id(const char *name)
+{
+	if (strncmp(name, XATTR_NAME_SMACK, sizeof(XATTR_NAME_SMACK) - 1))
+		return -EINVAL;
+
+	return smk_xa_suffix_to_id(name + (sizeof(XATTR_NAME_SMACK) - 1));
+}
+
+static int
+smk_xa_secname_to_id(const char *name)
+{
+	if (strncmp(name, XATTR_SMACK_SUFFIX, sizeof(XATTR_SMACK_SUFFIX) - 1))
+		return -EINVAL;
+
+	return smk_xa_suffix_to_id(name + (sizeof(XATTR_SMACK_SUFFIX) - 1));
+}
+
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
 static char *smk_bu_mess[] = {
 	"Bringup Error",	/* Unused */
@@ -1355,18 +1398,7 @@ static int smack_inode_getattr(const struct path *path)
  */
 static int smack_inode_xattr_skipcap(const char *name)
 {
-	if (strncmp(name, XATTR_NAME_SMACK, sizeof(XATTR_NAME_SMACK) - 1))
-		return 0;
-
-	if (strcmp(name, XATTR_NAME_SMACK) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKIPOUT) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKMMAP) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0)
-		return 1;
-
-	return 0;
+	return (smk_xa_name_to_id(name) >= 0);
 }
 
 /*
@@ -1406,15 +1438,17 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 	struct inode * const inode = d_backing_inode(dentry);
 	umode_t const i_mode = inode->i_mode;
 
-	if (strcmp(name, XATTR_NAME_SMACK) == 0) {
+	switch (smk_xa_name_to_id(name)) {
+	case SMK_XA_OBJECT:
 		/*
 		 * inode of socket file descriptor (sockfs inode) and
 		 * UDS inode have fixed label
 		 */
 		if (S_ISSOCK(i_mode))
 			return -EOPNOTSUPP;
-	} else if (strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
-		   strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
+		break;
+	case SMK_XA_IPIN:
+	case SMK_XA_IPOUT:
 		/*
 		 * inode of socket file descriptor (sockfs inode) only
 		 */
@@ -1423,19 +1457,22 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 
 		if (SOCKET_I(inode)->sk == NULL)
 			return -EOPNOTSUPP;
-	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
-		   strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
+		break;
+	case SMK_XA_EXEC:
+	case SMK_XA_MMAP:
 		if (!S_ISREG(i_mode))
 			return -EOPNOTSUPP;
 		task_label = true;
-	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
+		break;
+	case SMK_XA_TRANSMUTE:
 		if (!S_ISDIR(i_mode))
 			return -EOPNOTSUPP;
 		if (size != TRANS_TRUE_SIZE ||
 		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
 			return -EINVAL;
 		label_inside = false;
-	} else {
+		break;
+	default: {
 		/*
 		 * treat other xattrs as labeled data
 		 */
@@ -1447,6 +1484,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 		return smk_bu_inode(inode, MAY_WRITE,
 		       smk_curacc(smk_of_inode(inode), MAY_WRITE, &ad));
 	}
+	}
 
 	if (!smack_privileged(CAP_MAC_ADMIN))
 		return -EPERM;
@@ -1487,17 +1525,21 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name,
 	struct smack_known **skpp = NULL;
 	struct inode_smack *isp = smack_inode(d_backing_inode(dentry));
 
-	if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
+	switch (smk_xa_name_to_id(name)) {
+	case SMK_XA_TRANSMUTE:
 		isp->smk_flags |= SMK_INODE_TRANSMUTE;
 		return;
+	case SMK_XA_OBJECT:
+		skpp = &isp->smk_inode;
+		break;
+	case SMK_XA_EXEC:
+		skpp = &isp->smk_task;
+		break;
+	case SMK_XA_MMAP:
+		skpp = &isp->smk_mmap;
+		break;
 	}
 
-	if (strcmp(name, XATTR_NAME_SMACK) == 0)
-		skpp = &isp->smk_inode;
-	else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0)
-		skpp = &isp->smk_task;
-	else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0)
-		skpp = &isp->smk_mmap;
 	/*
 	 * Label has been imported by smack_inode_setxattr, just find it
 	 */
@@ -1545,16 +1587,9 @@ static int smack_inode_removexattr(struct mnt_idmap *idmap,
 {
 	const struct inode * const inode = d_backing_inode(dentry);
 	struct inode_smack * const isp = smack_inode(inode);
+	int const xa_id = smk_xa_name_to_id(name);
 
-	if (strcmp(name, XATTR_NAME_SMACK) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKIPOUT) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0 ||
-	    strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
-		if (!smack_privileged(CAP_MAC_ADMIN))
-			return -EPERM;
-	} else {
+	if (xa_id < 0) {
 		/*
 		 * treat other xattrs as labeled data
 		 */
@@ -1567,25 +1602,34 @@ static int smack_inode_removexattr(struct mnt_idmap *idmap,
 		       smk_curacc(isp->smk_inode, MAY_WRITE, &ad));
 	}
 
+	if (!smack_privileged(CAP_MAC_ADMIN))
+		return -EPERM;
+
 	/*
 	 * Don't do anything special for these.
 	 *	XATTR_NAME_SMACKIPIN
 	 *	XATTR_NAME_SMACKIPOUT
 	 *	XATTR_NAME_SMACK if S_ISSOCK (UDS inode has fixed label)
 	 */
-	if (strcmp(name, XATTR_NAME_SMACK) == 0) {
+	switch (xa_id) {
+	case SMK_XA_OBJECT:
 		if (!S_ISSOCK(inode->i_mode)) {
 			struct super_block *sbp = dentry->d_sb;
 			struct superblock_smack *sbsp = smack_superblock(sbp);
 
 			isp->smk_inode = sbsp->smk_default;
 		}
-	} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0)
+		break;
+	case SMK_XA_EXEC:
 		isp->smk_task = NULL;
-	else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0)
+		break;
+	case SMK_XA_MMAP:
 		isp->smk_mmap = NULL;
-	else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0)
+		break;
+	case SMK_XA_TRANSMUTE:
 		isp->smk_flags &= ~SMK_INODE_TRANSMUTE;
+		break;
+	}
 
 	return 0;
 }
@@ -1676,20 +1720,26 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 	const struct inode_smack * const isp = smack_inode(inode);
 	const char *value = NULL;
 	int value_len;
+	int const xa_id = smk_xa_secname_to_id(name);
 
-	if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
+	switch (xa_id) {
+	case SMK_XA_OBJECT:
 		skp = isp->smk_inode;
-	} else if (strcmp(name, XATTR_SMACK_EXEC) == 0) {
+		break;
+	case SMK_XA_EXEC:
 		skp = isp->smk_task;
-	} else if (strcmp(name, XATTR_SMACK_MMAP) == 0) {
+		break;
+	case SMK_XA_MMAP:
 		skp = isp->smk_mmap;
-	} else if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) {
+		break;
+	case SMK_XA_TRANSMUTE:
 		if (isp->smk_flags & SMK_INODE_TRANSMUTE)
 			value = TRANS_TRUE;
 		else
 			return -ENODATA;
-	} else if (strcmp(name, XATTR_SMACK_IPIN) == 0 ||
-		   strcmp(name, XATTR_SMACK_IPOUT) == 0) {
+		break;
+	case SMK_XA_IPIN:
+	case SMK_XA_IPOUT: {
 		/*
 		 * These Smack xattrs are only on sockets.
 		 */
@@ -1705,12 +1755,15 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
 
 		ssp = smack_sock(sk);
 
-		if (strcmp(name, XATTR_SMACK_IPIN) == 0)
+		if (xa_id == SMK_XA_IPIN)
 			skp = ssp->smk_in;
 		else
 			skp = ssp->smk_out;
-	} else
+		break;
+	}
+	default:
 		return -EOPNOTSUPP;
+	}
 
 	if (!value) {
 		if (!skp)
@@ -2986,28 +3039,23 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	struct socket_smack *ssp;
 	struct socket *sock;
 	int rc = 0;
+	int const xa_id = smk_xa_secname_to_id(name);
 
-	if (!(strcmp(name, XATTR_SMACK_SUFFIX) == 0 ||
-	      strcmp(name, XATTR_SMACK_TRANSMUTE) == 0 ||
-	      strcmp(name, XATTR_SMACK_EXEC) == 0 ||
-	      strcmp(name, XATTR_SMACK_MMAP) == 0 ||
-	      strcmp(name, XATTR_SMACK_IPIN) == 0 ||
-	      strcmp(name, XATTR_SMACK_IPOUT) == 0
-	))
+	if (xa_id < 0)
 		return -EOPNOTSUPP;
 
-	if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) {
+	switch (xa_id) {
+	case SMK_XA_TRANSMUTE:
 		nsp->smk_flags |= SMK_INODE_TRANSMUTE;
 		return 0;
-	}
-
-	if (strcmp(name, XATTR_SMACK_EXEC) == 0 ||
-	    strcmp(name, XATTR_SMACK_MMAP) == 0)
+	case SMK_XA_EXEC:
+	case SMK_XA_MMAP:
 		return -ENODATA;
+	}
 
 	skp = smk_find_label(value, size);
 
-	if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
+	if (xa_id == SMK_XA_OBJECT) {
 		nsp->smk_inode = skp;
 		nsp->smk_flags |= SMK_INODE_INSTANT;
 		return 0;
@@ -3020,7 +3068,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	sock = SOCKET_I(inode);
 	ssp = smack_sock(sock->sk);
 
-	if (strcmp(name, XATTR_SMACK_IPIN) == 0)
+	if (xa_id == SMK_XA_IPIN)
 		ssp->smk_in = skp;
 	else {
 		ssp->smk_out = skp;
-- 
2.43.0

Re: [PATCH 00/19] smack: clean up xattr handling

[Casey Schaufler]

On 7/24/2025 6:09 AM, Konstantin Andreev wrote:
> A set of minor bug fixes and optimizations in Smack xattr handling.
> Logically independent, but with the code dependencies.

Please break this into two (or more) patch sets. The patches regarding
restrictions on getting and setting the file type specific attributes
should be presented independently of the xattr "fixes".

There appears to be a misunderstanding regarding "valid" Smack labels.
A Smack label is a text string. The intention is that a label is "valid"
if the system is exposed to it. For example,

	# echo Oatmeal > /proc/self/attr/smack/current

should introduce "Oatmeal" as a Smack label if is has never been used
before. After a reboot the system may find the label "Bacon" on a file,
and if the label isn't known it is imported. Similarly, if a CIPSO packet
includes a label that has not been seen in is added.

This policy is necessary in part because there is a valid use case for
a Smack label with no explicit access rules.

I tried out the combined set and encountered many unexpected failures.

>
> The patch set applies on top of:
> https://github.com/cschaufler/smack-next/commits/next
> commit 6ddd169d0288
>
> Konstantin Andreev (19):
>   smack: fix bug: changing Smack xattrs requires cap_sys_admin
>   smack: fix bug: changing Smack xattrs requires cap_mac_override
>   smack: fix bug: setting label-containing xattrs silently ignores input garbage
>   smack: stop polling other LSMs & VFS to getxattr() unsupported SMACK64IPIN/OUT
>   smack: restrict getxattr() SMACK64TRANSMUTE to directories
>   smack: fix bug: getxattr() returns invalid SMACK64EXEC/MMAP
>   smack: deduplicate task label validation
>   smack: smack_inode_setsecurity: prevent setting SMACK64EXEC/MMAP in other LSMs
>   smack: smack_inode_setsecurity: prevent setting SMACK64IPIN/OUT in other LSMs
>   smack: fix bug: smack_inode_setsecurity() imports alien xattrs as labels
>   smack: fix bug: smack_inode_setsecurity() false EINVAL for alien xattrs
>   smack: restrict setxattr() SMACK64IPIN/IPOUT to sockets
>   smack: restrict setxattr() SMACK64EXEC/MMAP to regular files
>   smack: return EOPNOTSUPP for setxattr() unsupported SMACK64(TRANSMUTE)
>   smack: smack_inode_setsecurity(): skip checks for SMACK64TRANSMUTE
>   smack: smack_inode_notifysecctx(): reject invalid labels
>   smack: smack_inode_post_setxattr(): find label instead of import
>   smack: smack_inode_setsecurity(): find label instead of import
>   smack: deduplicate strcmp(name, XATTR_{,NAME_}SMACK*)
>
>  Documentation/admin-guide/LSM/Smack.rst |   3 +-
>  security/smack/smack.h                  |   2 +
>  security/smack/smack_access.c           |  22 +-
>  security/smack/smack_lsm.c              | 492 +++++++++++++++---------
>  4 files changed, 324 insertions(+), 195 deletions(-)
>

Re[2]: [PATCH 00/19] smack: clean up xattr handling

[Konstantin Andreev]

Casey Schaufler, 26 Jul 2025:
> On 7/24/2025 6:09 AM, Konstantin Andreev wrote:
>> A set of minor bug fixes and optimizations in Smack xattr handling.
>> Logically independent, but with the code dependencies.
> 
> Please break this into two (or more) patch sets. The patches regarding
> restrictions on getting and setting the file type specific attributes
> should be presented independently of the xattr "fixes".

Hi, Casey.

Each patch in the set is finite by itself,
addresses an independend issue, and I ought
to send them separately. There are several reasons
why I ended up to collect them into the set:

   1) they are very small and have common subject
   2) they have code dependencies
   3) the feedback time for Smack patches may be high,
      and sending patches one-by-one may be long.
   4) to give you an overview of the offered changes


May it be suitable for you to consider this set
as independent patches, that require, however,
the specific order of applying?

With this approach you could stop considering the
sequence at the 1st unacceptable/wrong/failed patch.

Else, I can collect them into two (or more) patch sets.

> There appears to be a misunderstanding regarding "valid" Smack labels.
> A Smack label is a text string. The intention is that a label is "valid"
> if the system is exposed to it. For example,
> 
> 	# echo Oatmeal > /proc/self/attr/smack/current
> 
> should introduce "Oatmeal" as a Smack label if is has never been used
> before. After a reboot the system may find the label "Bacon" on a file,
> and if the label isn't known it is imported.

I think I understand this.

> Similarly, if a CIPSO packet
> includes a label that has not been seen in is added.

I have never seen this. The unseen CIPSO label is not added,
instead, the incoming packet is marked with `*' label.

Just rechecked, it's so. E.g., tcp/ipv4 connection,
listener side writes the audit record like:

| lsm=SMACK fn=smack_socket_sock_rcv_skb action=denied subject="*" object="foo" requested=w ...

when incoming CIPSO has [bar 250/2,3,7,10,11,16,18,19,20,23]
at the initiator, but unseen at the listener side.

The documentation does not define either way of processing
of the unseen Cipso labels, but current implementation looks
reasonable, as sheer import of network-provided labels
has no limits and opens the door for denial of service.

> This policy is necessary in part because there is a valid use case for
> a Smack label with no explicit access rules.
> 
> I tried out the combined set and encountered many unexpected failures.
> 
[... skip ...]