* [RFC][PATCH 2/2] smack: Record transmuting in smk_transmuted
2023-05-08 17:02 [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity() Roberto Sassu
@ 2023-05-08 17:02 ` Roberto Sassu
2023-05-11 17:13 ` Casey Schaufler
2023-05-08 17:24 ` [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity() Casey Schaufler
2023-05-11 17:12 ` Casey Schaufler
2 siblings, 1 reply; 6+ messages in thread
From: Roberto Sassu @ 2023-05-08 17:02 UTC (permalink / raw)
To: casey, paul, jmorris, serge
Cc: linux-security-module, linux-kernel, zohar, linux-unionfs,
miklos, yoonjaeh, kamatam, mengcc, Roberto Sassu
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>
---
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;
}
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC][PATCH 2/2] smack: Record transmuting in smk_transmuted
2023-05-08 17:02 ` [RFC][PATCH 2/2] smack: Record transmuting in smk_transmuted Roberto Sassu
@ 2023-05-11 17:13 ` Casey Schaufler
0 siblings, 0 replies; 6+ messages in thread
From: Casey Schaufler @ 2023-05-11 17:13 UTC (permalink / raw)
To: Roberto Sassu, paul, jmorris, serge
Cc: linux-security-module, linux-kernel, zohar, linux-unionfs,
miklos, yoonjaeh, kamatam, mengcc, Roberto Sassu,
Casey Schaufler
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;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity()
2023-05-08 17:02 [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity() Roberto Sassu
2023-05-08 17:02 ` [RFC][PATCH 2/2] smack: Record transmuting in smk_transmuted Roberto Sassu
@ 2023-05-08 17:24 ` Casey Schaufler
2023-05-11 17:12 ` Casey Schaufler
2 siblings, 0 replies; 6+ messages in thread
From: Casey Schaufler @ 2023-05-08 17:24 UTC (permalink / raw)
To: Roberto Sassu, paul, jmorris, serge
Cc: linux-security-module, linux-kernel, zohar, linux-unionfs,
miklos, yoonjaeh, kamatam, mengcc, Roberto Sassu,
Casey Schaufler
On 5/8/2023 10:02 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> 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.
I will kick the tires on these patches. They look good at first glance.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.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 7a3e9ab137d..c7e37ed2799 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1463,10 +1463,19 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
> struct super_block *sbp;
> struct inode *ip = 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 = smack_inode(inode);
> + if (ispp->smk_flags & SMK_INODE_TRANSMUTE)
> + label = TRANS_TRUE;
> + else
> + label = "";
> + } else {
> /*
> * The rest of the Smack xattrs are only on sockets.
> */
> @@ -1488,13 +1497,18 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
> 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;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity()
2023-05-08 17:02 [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity() Roberto Sassu
2023-05-08 17:02 ` [RFC][PATCH 2/2] smack: Record transmuting in smk_transmuted Roberto Sassu
2023-05-08 17:24 ` [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity() Casey Schaufler
@ 2023-05-11 17:12 ` Casey Schaufler
2023-08-21 7:55 ` Munehisa Kamata
2 siblings, 1 reply; 6+ messages in thread
From: Casey Schaufler @ 2023-05-11 17:12 UTC (permalink / raw)
To: Roberto Sassu, paul, jmorris, serge
Cc: linux-security-module, linux-kernel, zohar, linux-unionfs,
miklos, yoonjaeh, kamatam, mengcc, Roberto Sassu,
Casey Schaufler
On 5/8/2023 10:02 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> 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>
Looks good. I have added to smack next.
> ---
> 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 7a3e9ab137d..c7e37ed2799 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1463,10 +1463,19 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
> struct super_block *sbp;
> struct inode *ip = 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 = smack_inode(inode);
> + if (ispp->smk_flags & SMK_INODE_TRANSMUTE)
> + label = TRANS_TRUE;
> + else
> + label = "";
> + } else {
> /*
> * The rest of the Smack xattrs are only on sockets.
> */
> @@ -1488,13 +1497,18 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
> 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;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC][PATCH 1/2] smack: Retrieve transmuting information in smack_inode_getsecurity()
2023-05-11 17:12 ` Casey Schaufler
@ 2023-08-21 7:55 ` Munehisa Kamata
0 siblings, 0 replies; 6+ messages in thread
From: Munehisa Kamata @ 2023-08-21 7:55 UTC (permalink / raw)
To: casey
Cc: jmorris, kamatam, linux-kernel, linux-security-module,
linux-unionfs, mengcc, miklos, paul, roberto.sassu,
roberto.sassu, serge, yoonjaeh, zohar
Hi Casey, Roberto
On Thu, 2023-05-11 17:12:50 +0000, Casey Schaufler wrote:
>
> On 5/8/2023 10:02 AM, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > 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>
>
> Looks good. I have added to smack next.
Do you have any objections to backporting these patches to -stable? If not,
I'll ask it in the stable list along with another overlayfs-related fix
387ef964460f ("Smack:- Use overlay inode label in smack_inode_copy_up()").
Thanks,
Munehisa
>
> > ---
> > 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 7a3e9ab137d..c7e37ed2799 100644
> > --- a/security/smack/smack_lsm.c
> > +++ b/security/smack/smack_lsm.c
> > @@ -1463,10 +1463,19 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
> > struct super_block *sbp;
> > struct inode *ip = 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 = smack_inode(inode);
> > + if (ispp->smk_flags & SMK_INODE_TRANSMUTE)
> > + label = TRANS_TRUE;
> > + else
> > + label = "";
> > + } else {
> > /*
> > * The rest of the Smack xattrs are only on sockets.
> > */
> > @@ -1488,13 +1497,18 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
> > 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;
> > }
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread