* [RFC][PATCH 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
2023-06-07 12:36 [RFC][PATCH 0/5] Smack transmute fixes Roberto Sassu
@ 2023-06-07 12:36 ` Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity() Roberto Sassu
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Roberto Sassu @ 2023-06-07 12:36 UTC (permalink / raw)
To: casey, paul, jmorris, serge
Cc: linux-kernel, linux-security-module, Roberto Sassu, stable
From: Roberto Sassu <roberto.sassu@huawei.com>
Since the SMACK64TRANSMUTE xattr makes sense only for directories, enforce
this restriction in smack_inode_setxattr().
Cc: stable@vger.kernel.org
Fixes: 5c6d1125f8db ("Smack: Transmute labels on specified directories") # v2.6.38.x
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/smack/smack_lsm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index b67d901ee74..53fc6a1034d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1260,7 +1260,8 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
check_star = 1;
} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
check_priv = 1;
- if (size != TRANS_TRUE_SIZE ||
+ if (!S_ISDIR(d_backing_inode(dentry)->i_mode) ||
+ size != TRANS_TRUE_SIZE ||
strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
rc = -EINVAL;
} else
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC][PATCH 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
2023-06-07 12:36 [RFC][PATCH 0/5] Smack transmute fixes Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr() Roberto Sassu
@ 2023-06-07 12:36 ` Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 3/5] smack: Always determine inode labels in smack_inode_init_security() Roberto Sassu
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Roberto Sassu @ 2023-06-07 12:36 UTC (permalink / raw)
To: casey, paul, jmorris, serge
Cc: linux-kernel, linux-security-module, Roberto Sassu, stable
From: Roberto Sassu <roberto.sassu@huawei.com>
If the SMACK64TRANSMUTE xattr is provided, and the inode is a directory,
update the in-memory inode flags by setting SMK_INODE_TRANSMUTE.
Cc: stable@vger.kernel.org
Fixes: 5c6d1125f8db ("Smack: Transmute labels on specified directories") # v2.6.38.x
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/smack/smack_lsm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 53fc6a1034d..162ca400f07 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2802,6 +2802,15 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
if (value == NULL || size > SMK_LONGLABEL || size == 0)
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;
+ }
+
skp = smk_import_entry(value, size);
if (IS_ERR(skp))
return PTR_ERR(skp);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC][PATCH 3/5] smack: Always determine inode labels in smack_inode_init_security()
2023-06-07 12:36 [RFC][PATCH 0/5] Smack transmute fixes Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr() Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity() Roberto Sassu
@ 2023-06-07 12:36 ` Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 4/5] smack: Initialize the in-memory inode " Roberto Sassu
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Roberto Sassu @ 2023-06-07 12:36 UTC (permalink / raw)
To: casey, paul, jmorris, serge
Cc: linux-kernel, linux-security-module, Roberto Sassu
From: Roberto Sassu <roberto.sassu@huawei.com>
The inode_init_security hook is already a good place to initialize the
in-memory inode. And that is also what SELinux does.
In preparation for this, move the existing smack_inode_init_security() code
outside the 'if (xattr)' condition, and set the xattr, if provided.
This change does not have any impact on the current code, since every time
security_inode_init_security() is called, the initxattr() callback is
passed and, thus, xattr is non-NULL.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/smack/smack_lsm.c | 78 +++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 38 deletions(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 162ca400f07..f7382448e12 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -948,49 +948,51 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count);
int may;
- if (xattr) {
- /*
- * If equal, transmuting already occurred in
- * smack_dentry_create_files_as(). No need to check again.
- */
- if (tsp->smk_task != tsp->smk_transmuted) {
- rcu_read_lock();
- may = smk_access_entry(skp->smk_known, dsp->smk_known,
- &skp->smk_rules);
- rcu_read_unlock();
- }
+ /*
+ * If equal, transmuting already occurred in
+ * smack_dentry_create_files_as(). No need to check again.
+ */
+ if (tsp->smk_task != tsp->smk_transmuted) {
+ rcu_read_lock();
+ may = smk_access_entry(skp->smk_known, dsp->smk_known,
+ &skp->smk_rules);
+ rcu_read_unlock();
+ }
+
+ /*
+ * In addition to having smk_task equal to smk_transmuted,
+ * if the access rule allows transmutation and the directory
+ * requests transmutation then by all means transmute.
+ * Mark the inode as changed.
+ */
+ if ((tsp->smk_task == tsp->smk_transmuted) ||
+ (may > 0 && ((may & MAY_TRANSMUTE) != 0) &&
+ smk_inode_transmutable(dir))) {
+ struct xattr *xattr_transmute;
/*
- * In addition to having smk_task equal to smk_transmuted,
- * if the access rule allows transmutation and the directory
- * requests transmutation then by all means transmute.
- * Mark the inode as changed.
+ * The caller of smack_dentry_create_files_as()
+ * should have overridden the current cred, so the
+ * inode label was already set correctly in
+ * smack_inode_alloc_security().
*/
- if ((tsp->smk_task == tsp->smk_transmuted) ||
- (may > 0 && ((may & MAY_TRANSMUTE) != 0) &&
- smk_inode_transmutable(dir))) {
- struct xattr *xattr_transmute;
-
- /*
- * The caller of smack_dentry_create_files_as()
- * should have overridden the current cred, so the
- * inode label was already set correctly in
- * smack_inode_alloc_security().
- */
- if (tsp->smk_task != tsp->smk_transmuted)
- isp = dsp;
- xattr_transmute = lsm_get_xattr_slot(xattrs, xattr_count);
- if (xattr_transmute) {
- xattr_transmute->value = kmemdup(TRANS_TRUE,
- TRANS_TRUE_SIZE, GFP_NOFS);
- if (xattr_transmute->value == NULL)
- return -ENOMEM;
-
- xattr_transmute->value_len = TRANS_TRUE_SIZE;
- xattr_transmute->name = XATTR_SMACK_TRANSMUTE;
- }
+ if (tsp->smk_task != tsp->smk_transmuted)
+ isp = dsp;
+
+ xattr_transmute = lsm_get_xattr_slot(xattrs, xattr_count);
+ if (xattr_transmute) {
+ xattr_transmute->value = kmemdup(TRANS_TRUE,
+ TRANS_TRUE_SIZE,
+ GFP_NOFS);
+ if (xattr_transmute->value == NULL)
+ return -ENOMEM;
+
+ xattr_transmute->value_len = TRANS_TRUE_SIZE;
+ xattr_transmute->name = XATTR_SMACK_TRANSMUTE;
}
+ }
+ if (xattr) {
xattr->value = kstrdup(isp->smk_known, GFP_NOFS);
if (xattr->value == NULL)
return -ENOMEM;
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC][PATCH 4/5] smack: Initialize the in-memory inode in smack_inode_init_security()
2023-06-07 12:36 [RFC][PATCH 0/5] Smack transmute fixes Roberto Sassu
` (2 preceding siblings ...)
2023-06-07 12:36 ` [RFC][PATCH 3/5] smack: Always determine inode labels in smack_inode_init_security() Roberto Sassu
@ 2023-06-07 12:36 ` Roberto Sassu
2023-06-07 12:36 ` [RFC][PATCH 5/5] ramfs: Initialize security of in-memory inodes Roberto Sassu
2023-06-07 14:53 ` [RFC][PATCH 0/5] Smack transmute fixes Casey Schaufler
5 siblings, 0 replies; 7+ messages in thread
From: Roberto Sassu @ 2023-06-07 12:36 UTC (permalink / raw)
To: casey, paul, jmorris, serge
Cc: linux-kernel, linux-security-module, Roberto Sassu
From: Roberto Sassu <roberto.sassu@huawei.com>
Currently, Smack initializes in-memory new inodes in three steps. It first
sets the xattrs in smack_inode_init_security(), fetches them in
smack_d_instantiate() and finally, in the same function, sets the in-memory
inodes depending on xattr values, unless they are in specially-handled
filesystems.
Other than being inefficient, this also prevents filesystems not supporting
xattrs from working properly since, without xattrs, there is no way to pass
the label determined in smack_inode_init_security() to
smack_d_instantiate().
Since the LSM infrastructure allows setting and getting the security field
without xattrs through the inode_setsecurity and inode_getsecurity hooks,
make the inode creation work too, by initializing the in-memory inode
earlier in smack_inode_init_security().
Also mark the inode as instantiated, to prevent smack_d_instantiate() from
overwriting the security field. As mentioned above, this potentially has
impact for inodes in specially-handled filesystems in
smack_d_instantiate(), if they are not handled in the same way in
smack_inode_init_security().
Filesystems other than tmpfs don't call security_inode_init_security(), so
they would be always initialized in smack_d_instantiate(), as before. For
tmpfs, the current behavior is to assign to inodes the label '*', but
actually that label is overwritten with the one fetched from the SMACK64
xattr, set in smack_inode_init_security() (default: '_').
Initializing the in-memory inode is straightforward: if not transmuting,
nothing more needs to be done; if transmuting, overwrite the current inode
label with the one from the parent directory, and set SMK_INODE_TRANSMUTE.
Finally, set SMK_INODE_INSTANT for all cases, to mark the inode as
instantiated.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/smack/smack_lsm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index f7382448e12..1373c6d49ff 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -942,6 +942,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
struct xattr *xattrs, int *xattr_count)
{
struct task_smack *tsp = smack_cred(current_cred());
+ struct inode_smack *issp = smack_inode(inode);
struct smack_known *skp = smk_of_task(tsp);
struct smack_known *isp = smk_of_inode(inode);
struct smack_known *dsp = smk_of_inode(dir);
@@ -977,7 +978,9 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
* smack_inode_alloc_security().
*/
if (tsp->smk_task != tsp->smk_transmuted)
- isp = dsp;
+ isp = issp->smk_inode = dsp;
+
+ issp->smk_flags |= SMK_INODE_TRANSMUTE;
xattr_transmute = lsm_get_xattr_slot(xattrs, xattr_count);
if (xattr_transmute) {
@@ -992,6 +995,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
}
}
+ issp->smk_flags |= SMK_INODE_INSTANT;
+
if (xattr) {
xattr->value = kstrdup(isp->smk_known, GFP_NOFS);
if (xattr->value == NULL)
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC][PATCH 5/5] ramfs: Initialize security of in-memory inodes
2023-06-07 12:36 [RFC][PATCH 0/5] Smack transmute fixes Roberto Sassu
` (3 preceding siblings ...)
2023-06-07 12:36 ` [RFC][PATCH 4/5] smack: Initialize the in-memory inode " Roberto Sassu
@ 2023-06-07 12:36 ` Roberto Sassu
2023-06-07 14:53 ` [RFC][PATCH 0/5] Smack transmute fixes Casey Schaufler
5 siblings, 0 replies; 7+ messages in thread
From: Roberto Sassu @ 2023-06-07 12:36 UTC (permalink / raw)
To: casey, paul, jmorris, serge
Cc: linux-kernel, linux-security-module, Roberto Sassu
From: Roberto Sassu <roberto.sassu@huawei.com>
Add a call security_inode_init_security() after ramfs_get_inode(), to let
LSMs initialize the inode security field. Skip ramfs_fill_super(), as the
initialization is done through the sb_set_mnt_opts hook.
Calling security_inode_init_security() call inside ramfs_get_inode() is
not possible since, for CONFIG_SHMEM=n, tmpfs also calls the former after
the latter.
Pass NULL as initxattrs() callback to security_inode_init_security(), since
the purpose of the call is only to initialize the in-memory inodes.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
fs/ramfs/inode.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index 5ba580c7883..e6b5f04b2b2 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
int error = -ENOSPC;
if (inode) {
+ error = security_inode_init_security(inode, dir,
+ &dentry->d_name, NULL,
+ NULL);
+ if (error && error != -EOPNOTSUPP) {
+ iput(inode);
+ return error;
+ }
+
d_instantiate(dentry, inode);
dget(dentry); /* Extra count - pin the dentry in core */
error = 0;
@@ -134,6 +142,15 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK|S_IRWXUGO, 0);
if (inode) {
int l = strlen(symname)+1;
+
+ error = security_inode_init_security(inode, dir,
+ &dentry->d_name, NULL,
+ NULL);
+ if (error && error != -EOPNOTSUPP) {
+ iput(inode);
+ return error;
+ }
+
error = page_symlink(inode, symname, l);
if (!error) {
d_instantiate(dentry, inode);
@@ -149,10 +166,20 @@ static int ramfs_tmpfile(struct mnt_idmap *idmap,
struct inode *dir, struct file *file, umode_t mode)
{
struct inode *inode;
+ int error;
inode = ramfs_get_inode(dir->i_sb, dir, mode, 0);
if (!inode)
return -ENOSPC;
+
+ error = security_inode_init_security(inode, dir,
+ &file_dentry(file)->d_name, NULL,
+ NULL);
+ if (error && error != -EOPNOTSUPP) {
+ iput(inode);
+ return error;
+ }
+
d_tmpfile(file, inode);
return finish_open_simple(file, 0);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC][PATCH 0/5] Smack transmute fixes
2023-06-07 12:36 [RFC][PATCH 0/5] Smack transmute fixes Roberto Sassu
` (4 preceding siblings ...)
2023-06-07 12:36 ` [RFC][PATCH 5/5] ramfs: Initialize security of in-memory inodes Roberto Sassu
@ 2023-06-07 14:53 ` Casey Schaufler
5 siblings, 0 replies; 7+ messages in thread
From: Casey Schaufler @ 2023-06-07 14:53 UTC (permalink / raw)
To: Roberto Sassu, paul, jmorris, serge
Cc: linux-kernel, linux-security-module, Roberto Sassu, Casey Schaufler
On 6/7/2023 5:36 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> The first two patches are obvious fixes, the first restricts setting the
> SMACK64TRANSMUTE xattr only for directories, and the second makes it
> possible to set SMACK64TRANSMUTE if the filesystem does not support xattrs
> (e.g. ramfs).
>
> The remaining fixes are optional, and only required if we want filesystems
> without xattr support behave like those with xattr support. Since we have
> the inode_setsecurity and inode_getsecurity hooks to make the first group
> work, it seems useful to fix inode creation too (SELinux should be fine).
>
> The third patch is merely a code move out of the 'if (xattr)' condition.
> The fourth updates the security field of the in-memory inode directly in
> smack_inode_init_security() and marks the inode as instantiated, and the
> fifth adds a security_inode_init_security() call in ramfs to initialize the
> security field of the in-memory inodes (needed to test transmuting
> directories).
>
> Both the Smack (on xfs) and IMA test suite succeed with all patches
> applied.
>
> By setting the ROOT variable to a ramfs mountpoint, the results are:
>
> Without the patches:
> 86 Passed, 9 Failed, 90% Success rate
>
> With the patches:
> 93 Passed, 2 Failed, 97% Success rate
>
> The remaining two failures are:
> 2151 ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported)
> 2152 lsetxattr("./targets/proc-attr-Snap", "security.SMACK64EXEC", "Pop", 3, 0) = -1 EOPNOTSUPP (Operation not supported)
>
> The first one is likely due ramfs lack of support for ioctl() while the
> second could be fixed by handling SMACK64EXEC in smack_inode_setsecurity().
>
> The patch set applies on top of:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/log/?h=next
>
> plus:
>
> https://github.com/cschaufler/smack-next/commits/next
>
> plus:
>
> https://lore.kernel.org/linux-integrity/20230603191518.1397490-1-roberto.sassu@huaweicloud.com/
>
> The ramfs patch potentially could be useful to correctly initialize the
> label of new inodes in the initramfs, assuming that it will be fully
> labeled with support for xattrs in the cpio image:
>
> https://lore.kernel.org/linux-integrity/20190523121803.21638-1-roberto.sassu@huawei.com/
>
> Ramfs inode labels will be set from xattrs with the inode_setsecurity hook.
>
> Roberto Sassu (5):
> smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
> smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
> smack: Always determine inode labels in smack_inode_init_security()
> smack: Initialize the in-memory inode in smack_inode_init_security()
> ramfs: Initialize security of in-memory inodes
>
> fs/ramfs/inode.c | 27 +++++++++++
> security/smack/smack_lsm.c | 93 ++++++++++++++++++++++----------------
> 2 files changed, 82 insertions(+), 38 deletions(-)
I will run these through my test cycle, but they look good at first glance.
Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread