Hi stable maintainers,
The following upstream commits fix a couple of overlayfs handling problems in Smack. Could you please cherry-pick them for 4.19.y, 5.4.y, 5.10.y, 5.15.y and 6.1.y?
#3 2c085f3a8f23 smack: Record transmuting in smk_transmuted #2 3a3d8fce31a4 smack: Retrieve transmuting information in smack_inode_getsecurity() #1 387ef964460f Smack:- Use overlay inode label in smack_inode_copy_up() # not needed for 6.1.y
Note that 4.19.y needs some adjustments. I'll send backported patches separately.
I'm not an author of these commits, but have been hitting the problems with multiple kernels based on the trees and it seems worth cherry-picking. Perhaps, it's better to add a tag in cherry-picked commits:
Fixes: d6d80cb57be4 ("Smack: Base support for overlayfs")
Regards, Munehisa
This series backports the following fixes for Smack problems with overlayfs to 4.19.y.
2c085f3a8f23 smack: Record transmuting in smk_transmuted 3a3d8fce31a4 smack: Retrieve transmuting information in smack_inode_getsecurity() 387ef964460f Smack:- Use overlay inode label in smack_inode_copy_up()
This slightly modifies the original commits, because the commits rely on some helper functions introduced after v4.19 by different commits that touch more code than just Smack, require even more prerequisite commits and also need some adjustments for 4.19.y. Instead, this series makes minor modifications for only the overlayfs-related fixes to not use the helper functions rather than backporting everything.
For reference, the upstream commits listed below introduced the helper functions. Though, this is not a complete list for their dependencies.
ecd5f82e05dd LSM: Infrastructure management of the ipc security blob 019bcca4626a Smack: Abstract use of ipc security blobs afb1cbe37440 LSM: Infrastructure management of the inode security fb4021b6fb58 Smack: Abstract use of inode security blob 33bf60cabcc7 LSM: Infrastructure management of the file security f28952ac9008 Smack: Abstract use of file security blob bbd3662a8348 Infrastructure management of the cred security blob b17103a8b8ae Smack: Abstract use of cred security blob
Roberto Sassu (2): smack: Retrieve transmuting information in smack_inode_getsecurity() smack: Record transmuting in smk_transmuted
Vishal Goel (1): Smack:- Use overlay inode label in smack_inode_copy_up()
security/smack/smack.h | 1 + security/smack/smack_lsm.c | 65 ++++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 17 deletions(-)
From: Vishal Goel vishal.goel@samsung.com
commit 387ef964460f14fe1c1ea29aba70e22731ea7cf7 upstream.
Currently in "smack_inode_copy_up()" function, process label is changed with the label on parent inode. Due to which, process is assigned directory label and whatever file or directory created by the process are also getting directory label which is wrong label.
Changes has been done to use label of overlay inode instead of parent inode.
Signed-off-by: Vishal Goel vishal.goel@samsung.com Signed-off-by: Casey Schaufler casey@schaufler-ca.com [4.19: adjusted for the lack of helper functions] Fixes: d6d80cb57be4 ("Smack: Base support for overlayfs") Signed-off-by: Munehisa Kamata kamatam@amazon.com --- 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 4f65d953fe31..a09a9c6bbdf6 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4612,7 +4612,7 @@ static int smack_inode_copy_up(struct dentry *dentry, struct cred **new) /* * Get label from overlay inode and set it in create_sid */ - isp = d_inode(dentry->d_parent)->i_security; + isp = d_inode(dentry)->i_security; skp = isp->smk_inode; tsp->smk_task = skp; *new = new_creds;
From: Roberto Sassu roberto.sassu@huawei.com
commit 3a3d8fce31a49363cc31880dce5e3b0617c9c38b upstream.
Enhance smack_inode_getsecurity() to retrieve the value for SMACK64TRANSMUTE from the inode security blob, similarly to SMACK64.
This helps to display accurate values in the situation where the security labels come from mount options and not from xattrs.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Signed-off-by: Casey Schaufler casey@schaufler-ca.com [4.19: adjusted for the lack of helper functions] Fixes: d6d80cb57be4 ("Smack: Base support for overlayfs") Signed-off-by: Munehisa Kamata kamatam@amazon.com --- security/smack/smack_lsm.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index a09a9c6bbdf6..db729834d8ba 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1490,10 +1490,19 @@ static int smack_inode_getsecurity(struct inode *inode, struct super_block *sbp; struct inode *ip = (struct inode *)inode; struct smack_known *isp; + struct inode_smack *ispp; + size_t label_len; + char *label = NULL;
- if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) + if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) { isp = smk_of_inode(inode); - else { + } else if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) { + ispp = inode->i_security; + if (ispp->smk_flags & SMK_INODE_TRANSMUTE) + label = TRANS_TRUE; + else + label = ""; + } else { /* * The rest of the Smack xattrs are only on sockets. */ @@ -1515,13 +1524,18 @@ static int smack_inode_getsecurity(struct inode *inode, return -EOPNOTSUPP; }
+ if (!label) + label = isp->smk_known; + + label_len = strlen(label); + if (alloc) { - *buffer = kstrdup(isp->smk_known, GFP_KERNEL); + *buffer = kstrdup(label, GFP_KERNEL); if (*buffer == NULL) return -ENOMEM; }
- return strlen(isp->smk_known); + return label_len; }
From: Roberto Sassu roberto.sassu@huawei.com
commit 2c085f3a8f23c9b444e8b99d93c15d7ce870fc4e upstream.
smack_dentry_create_files_as() determines whether transmuting should occur based on the label of the parent directory the new inode will be added to, and not the label of the directory where it is created.
This helps for example to do transmuting on overlayfs, since the latter first creates the inode in the working directory, and then moves it to the correct destination.
However, despite smack_dentry_create_files_as() provides the correct label, smack_inode_init_security() does not know from passed information whether or not transmuting occurred. Without this information, smack_inode_init_security() cannot set SMK_INODE_CHANGED in smk_flags, which will result in the SMACK64TRANSMUTE xattr not being set in smack_d_instantiate().
Thus, add the smk_transmuted field to the task_smack structure, and set it in smack_dentry_create_files_as() to smk_task if transmuting occurred. If smk_task is equal to smk_transmuted in smack_inode_init_security(), act as if transmuting was successful but without taking the label from the parent directory (the inode label was already set correctly from the current credentials in smack_inode_alloc_security()).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Signed-off-by: Casey Schaufler casey@schaufler-ca.com [4.19: adjusted for the lack of helper functions] Fixes: d6d80cb57be4 ("Smack: Base support for overlayfs") Signed-off-by: Munehisa Kamata kamatam@amazon.com --- security/smack/smack.h | 1 + security/smack/smack_lsm.c | 41 +++++++++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/security/smack/smack.h b/security/smack/smack.h index f7db791fb566..62aa4bc25426 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -120,6 +120,7 @@ struct inode_smack { struct task_smack { struct smack_known *smk_task; /* label for access control */ struct smack_known *smk_forked; /* label when forked */ + struct smack_known *smk_transmuted;/* label when transmuted */ struct list_head smk_rules; /* per task access rules */ struct mutex smk_rules_lock; /* lock for the rules */ struct list_head smk_relabel; /* transit allowed labels */ diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index db729834d8ba..266eb8ca3381 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1032,8 +1032,9 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, const char **name, void **value, size_t *len) { + struct task_smack *tsp = current_security(); struct inode_smack *issp = inode->i_security; - struct smack_known *skp = smk_of_current(); + struct smack_known *skp = smk_of_task(tsp); struct smack_known *isp = smk_of_inode(inode); struct smack_known *dsp = smk_of_inode(dir); int may; @@ -1042,20 +1043,34 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, *name = XATTR_SMACK_SUFFIX;
if (value && len) { - 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(); + }
/* - * If the access rule allows transmutation and - * the directory requests transmutation then - * by all means 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. */ - if (may > 0 && ((may & MAY_TRANSMUTE) != 0) && - smk_inode_transmutable(dir)) { - isp = dsp; + if ((tsp->smk_task == tsp->smk_transmuted) || + (may > 0 && ((may & MAY_TRANSMUTE) != 0) && + smk_inode_transmutable(dir))) { + /* + * 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; issp->smk_flags |= SMK_INODE_CHANGED; }
@@ -4677,8 +4692,10 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode, * providing access is transmuting use the containing * directory label instead of the process label. */ - if (may > 0 && (may & MAY_TRANSMUTE)) + if (may > 0 && (may & MAY_TRANSMUTE)) { ntsp->smk_task = isp->smk_inode; + ntsp->smk_transmuted = ntsp->smk_task; + } } return 0; }
Hi Sasha,
On Tue, 2023-10-03 11:26:57 +0000, Sasha Levin wrote:
Sorry if it was not clear enough in the first post[1]. For 5.4+, please just cherry-pick the 3 commits. Those should apply cleanly.
[1] https://lore.kernel.org/stable/20230929015033.835263-1-kamatam@amazon.com/
Thanks, Munehisa
linux-stable-mirror@lists.linaro.org
-
Munehisa Kamata -
Sasha Levin