On 5/8/2023 10:02 AM, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > 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@xxxxxxxxxx> Added to smack next. > --- > 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 e2239be7bd6..aa15ff56ed6 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 c7e37ed2799..6e270cf3fd3 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -933,8 +933,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 = smack_cred(current_cred()); > struct inode_smack *issp = smack_inode(inode); > - 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; > @@ -943,20 +944,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; > } > > @@ -4767,8 +4782,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; > }