| Message ID | 20230508170234.3595105-2-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [RFC,1/2] smack: Retrieve transmuting information in smack_inode_getsecurity() | expand |
On 5/8/2023 10:02 AM, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > 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> 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; > }
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; }