6.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Konstantin Andreev andreev@swemel.ru
[ Upstream commit 78fc6a94be252b27bb73e4926eed70b5e302a8e0 ]
According to [1], the label of a UNIX domain socket (UDS) file (i.e., the filesystem object representing the socket) is not supposed to participate in Smack security.
To achieve this, [1] labels UDS files with "*" in smack_d_instantiate().
Before [2], smack_d_instantiate() was responsible for initializing Smack security for all inodes, except ones under /proc
[2] imposed the sole responsibility for initializing inode security for newly created filesystem objects on smack_inode_init_security().
However, smack_inode_init_security() lacks some logic present in smack_d_instantiate(). In particular, it does not label UDS files with "*".
This patch adds the missing labeling of UDS files with "*" to smack_inode_init_security().
Labeling UDS files with "*" in smack_d_instantiate() still works for stale UDS files that already exist on disk. Stale UDS files are useless, but I keep labeling them for consistency and maybe to make easier for user to delete them.
Compared to [1], this version introduces the following improvements:
* UDS file label is held inside inode only and not saved to xattrs.
* relabeling UDS files (setxattr, removexattr, etc.) is blocked.
[1] 2010-11-24 Casey Schaufler commit b4e0d5f0791b ("Smack: UDS revision")
[2] 2023-11-16 roberto.sassu Fixes: e63d86b8b764 ("smack: Initialize the in-memory inode in smack_inode_init_security()") Link: https://lore.kernel.org/linux-security-module/20231116090125.187209-5-robert...
Signed-off-by: Konstantin Andreev andreev@swemel.ru Signed-off-by: Casey Schaufler casey@schaufler-ca.com Signed-off-by: Sasha Levin sashal@kernel.org --- Documentation/admin-guide/LSM/Smack.rst | 5 +++ security/smack/smack_lsm.c | 58 +++++++++++++++++++------ 2 files changed, 49 insertions(+), 14 deletions(-)
diff --git a/Documentation/admin-guide/LSM/Smack.rst b/Documentation/admin-guide/LSM/Smack.rst index 6d44f4fdbf59f..1b554b5bf98e6 100644 --- a/Documentation/admin-guide/LSM/Smack.rst +++ b/Documentation/admin-guide/LSM/Smack.rst @@ -696,6 +696,11 @@ sockets. A privileged program may set this to match the label of another task with which it hopes to communicate.
+UNIX domain socket (UDS) with a BSD address functions both as a file in a +filesystem and as a socket. As a file, it carries the SMACK64 attribute. This +attribute is not involved in Smack security enforcement and is immutably +assigned the label "*". + Smack Netlabel Exceptions ~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index db2a9aa9f1a05..d16453801a79e 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1020,6 +1020,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, bool trans_cred; bool trans_rule;
+ /* + * UNIX domain sockets use lower level socket data. Let + * UDS inode have fixed * label to keep smack_inode_permission() calm + * when called from unix_find_bsd() + */ + if (S_ISSOCK(inode->i_mode)) { + /* forced label, no need to save to xattrs */ + issp->smk_inode = &smack_known_star; + goto instant_inode; + } /* * If equal, transmuting already occurred in * smack_dentry_create_files_as(). No need to check again. @@ -1056,14 +1066,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, } }
- issp->smk_flags |= (SMK_INODE_INSTANT | transflag); - if (rc) - return rc; - - return xattr_dupval(xattrs, xattr_count, + if (rc == 0) + if (xattr_dupval(xattrs, xattr_count, XATTR_SMACK_SUFFIX, issp->smk_inode->smk_known, - strlen(issp->smk_inode->smk_known)); + strlen(issp->smk_inode->smk_known) + )) + rc = -ENOMEM; +instant_inode: + issp->smk_flags |= (SMK_INODE_INSTANT | transflag); + return rc; }
/** @@ -1337,13 +1349,23 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap, int check_import = 0; int check_star = 0; int rc = 0; + umode_t const i_mode = d_backing_inode(dentry)->i_mode;
/* * Check label validity here so import won't fail in post_setxattr */ - if (strcmp(name, XATTR_NAME_SMACK) == 0 || - strcmp(name, XATTR_NAME_SMACKIPIN) == 0 || - strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) { + if (strcmp(name, XATTR_NAME_SMACK) == 0) { + /* + * UDS inode has fixed label + */ + if (S_ISSOCK(i_mode)) { + rc = -EINVAL; + } else { + check_priv = 1; + 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 || @@ -1353,7 +1375,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap, check_star = 1; } else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) { check_priv = 1; - if (!S_ISDIR(d_backing_inode(dentry)->i_mode) || + if (!S_ISDIR(i_mode) || size != TRANS_TRUE_SIZE || strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0) rc = -EINVAL; @@ -1484,12 +1506,15 @@ static int smack_inode_removexattr(struct mnt_idmap *idmap, * 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) { - struct super_block *sbp = dentry->d_sb; - struct superblock_smack *sbsp = smack_superblock(sbp); + if (!S_ISSOCK(d_backing_inode(dentry)->i_mode)) { + struct super_block *sbp = dentry->d_sb; + struct superblock_smack *sbsp = smack_superblock(sbp);
- isp->smk_inode = sbsp->smk_default; + isp->smk_inode = sbsp->smk_default; + } } else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0) isp->smk_task = NULL; else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0) @@ -3607,7 +3632,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) */
/* - * UNIX domain sockets use lower level socket data. + * UDS inode has fixed label (*) */ if (S_ISSOCK(inode->i_mode)) { final = &smack_known_star; @@ -4872,6 +4897,11 @@ 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) { + /* + * UDS inode has fixed label. Ignore nfs label. + */ + if (S_ISSOCK(inode->i_mode)) + return 0; return smack_inode_setsecurity(inode, XATTR_SMACK_SUFFIX, ctx, ctxlen, 0); }