| Message ID | 20230603191518.1397490-3-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | evm: Do HMAC of multiple per LSM xattrs for new inodes | expand |
On Sat, 2023-06-03 at 21:15 +0200, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > With the newly added ability of LSMs to supply multiple xattrs, set > SMACK64TRASMUTE in smack_inode_init_security(), instead of d_instantiate(). > Do it by incrementing SMACK_INODE_INIT_XATTRS to 2 and by calling > lsm_get_xattr_slot() a second time, if the transmuting conditions are met. > > The LSM infrastructure passes all xattrs provided by LSMs to the > filesystems through the initxattrs() callback, so that filesystems can > store xattrs in the disk. > > After the change, the SMK_INODE_TRANSMUTE inode flag is always set by > d_instantiate() after fetching SMACK64TRANSMUTE from the disk. Before it > was done by smack_inode_post_setxattr() as result of the __vfs_setxattr() > call. > > Removing __vfs_setxattr() also prevents invalidating the EVM HMAC, by > adding a new xattr without checking and updating the existing HMAC. Hi Mengchi could you please redo your tests with this patch set applied? https://lore.kernel.org/linux-integrity/20230603191518.1397490-1-roberto.sassu@huaweicloud.com/ You need: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/log/?h=next https://github.com/cschaufler/smack-next/commits/next Thanks Roberto > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > --- > security/smack/smack.h | 2 +- > security/smack/smack_lsm.c | 43 +++++++++++++++++++++++--------------- > 2 files changed, 27 insertions(+), 18 deletions(-) > > diff --git a/security/smack/smack.h b/security/smack/smack.h > index aa15ff56ed6..041688e5a77 100644 > --- a/security/smack/smack.h > +++ b/security/smack/smack.h > @@ -128,7 +128,7 @@ struct task_smack { > > #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ > #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ > -#define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ > +#define SMK_INODE_CHANGED 0x04 /* smack was transmuted (unused) */ > #define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ > > /* > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index a1c30275692..b67d901ee74 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -52,7 +52,14 @@ > #define SMK_RECEIVING 1 > #define SMK_SENDING 2 > > -#define SMACK_INODE_INIT_XATTRS 1 > +/* > + * Smack uses multiple xattrs. > + * SMACK64 - for access control, > + * SMACK64TRANSMUTE - label initialization, > + * Not saved on files - SMACK64IPIN and SMACK64IPOUT, > + * Must be set explicitly - SMACK64EXEC and SMACK64MMAP > + */ > +#define SMACK_INODE_INIT_XATTRS 2 > > #ifdef SMACK_IPV6_PORT_LABELING > static DEFINE_MUTEX(smack_ipv6_lock); > @@ -935,7 +942,6 @@ 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); > @@ -963,6 +969,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, > 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 > @@ -971,7 +979,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, > */ > if (tsp->smk_task != tsp->smk_transmuted) > isp = dsp; > - issp->smk_flags |= SMK_INODE_CHANGED; > + 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; > + } > } > > xattr->value = kstrdup(isp->smk_known, GFP_NOFS); > @@ -3518,20 +3535,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) > * If there is a transmute attribute on the > * directory mark the inode. > */ > - if (isp->smk_flags & SMK_INODE_CHANGED) { > - isp->smk_flags &= ~SMK_INODE_CHANGED; > - rc = __vfs_setxattr(&nop_mnt_idmap, dp, inode, > - XATTR_NAME_SMACKTRANSMUTE, > - TRANS_TRUE, TRANS_TRUE_SIZE, > - 0); > - } else { > - rc = __vfs_getxattr(dp, inode, > - XATTR_NAME_SMACKTRANSMUTE, trattr, > - TRANS_TRUE_SIZE); > - if (rc >= 0 && strncmp(trattr, TRANS_TRUE, > - TRANS_TRUE_SIZE) != 0) > - rc = -EINVAL; > - } > + rc = __vfs_getxattr(dp, inode, > + XATTR_NAME_SMACKTRANSMUTE, trattr, > + TRANS_TRUE_SIZE); > + if (rc >= 0 && strncmp(trattr, TRANS_TRUE, > + TRANS_TRUE_SIZE) != 0) > + rc = -EINVAL; > if (rc >= 0) > transflag = SMK_INODE_TRANSMUTE; > }
On Mon Jun 5, 2023 at 11:38 AM EEST, Roberto Sassu wrote: > On Sat, 2023-06-03 at 21:15 +0200, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > With the newly added ability of LSMs to supply multiple xattrs, set > > SMACK64TRASMUTE in smack_inode_init_security(), instead of d_instantiate(). nit: TRANSMUTE Sorry, just hit into my eye. I skimmed it because I implemented original feature :-) BR, Jarkko
Hi Roberto, On Sat, 2023-06-03 at 21:15 +0200, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > With the newly added ability of LSMs to supply multiple xattrs, set > SMACK64TRASMUTE in smack_inode_init_security(), instead of d_instantiate(). > Do it by incrementing SMACK_INODE_INIT_XATTRS to 2 and by calling > lsm_get_xattr_slot() a second time, if the transmuting conditions are met. > > The LSM infrastructure passes all xattrs provided by LSMs to the > filesystems through the initxattrs() callback, so that filesystems can > store xattrs in the disk. > > After the change, the SMK_INODE_TRANSMUTE inode flag is always set by > d_instantiate() after fetching SMACK64TRANSMUTE from the disk. Before it > was done by smack_inode_post_setxattr() as result of the __vfs_setxattr() > call. > > Removing __vfs_setxattr() also prevents invalidating the EVM HMAC, by > adding a new xattr without checking and updating the existing HMAC. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Just a few comments/nits inline. > --- > security/smack/smack.h | 2 +- > security/smack/smack_lsm.c | 43 +++++++++++++++++++++++--------------- > 2 files changed, 27 insertions(+), 18 deletions(-) > > diff --git a/security/smack/smack.h b/security/smack/smack.h > index aa15ff56ed6..041688e5a77 100644 > --- a/security/smack/smack.h > +++ b/security/smack/smack.h > @@ -128,7 +128,7 @@ struct task_smack { > > #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ > #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ > -#define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ > +#define SMK_INODE_CHANGED 0x04 /* smack was transmuted (unused) */ > #define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ > > /* > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index a1c30275692..b67d901ee74 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -52,7 +52,14 @@ > #define SMK_RECEIVING 1 > #define SMK_SENDING 2 > > -#define SMACK_INODE_INIT_XATTRS 1 > +/* > + * Smack uses multiple xattrs. > + * SMACK64 - for access control, > + * SMACK64TRANSMUTE - label initialization, > + * Not saved on files - SMACK64IPIN and SMACK64IPOUT, > + * Must be set explicitly - SMACK64EXEC and SMACK64MMAP > + */ > +#define SMACK_INODE_INIT_XATTRS 2 > > #ifdef SMACK_IPV6_PORT_LABELING > static DEFINE_MUTEX(smack_ipv6_lock); > @@ -935,7 +942,6 @@ 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); > @@ -963,6 +969,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, > if ((tsp->smk_task == tsp->smk_transmuted) || > (may > 0 && ((may & MAY_TRANSMUTE) != 0) && > smk_inode_transmutable(dir))) { > + struct xattr *xattr_transmute; > + Variables should be defined at the beginning of the function. Is there a reason for beginning the function with "if (xattr) {" instead "if (!xattr) return 0;". This causes unnecessary indenting. > /* > * The caller of smack_dentry_create_files_as() > * should have overridden the current cred, so the > @@ -971,7 +979,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, > */ > if (tsp->smk_task != tsp->smk_transmuted) > isp = dsp; > - issp->smk_flags |= SMK_INODE_CHANGED; > + xattr_transmute = lsm_get_xattr_slot(xattrs, xattr_count); > + if (xattr_transmute) { > + xattr_transmute->value = kmemdup(TRANS_TRUE, > + TRANS_TRUE_SIZE, GFP_NOFS); script/checkpatch --strict complains here. > + if (xattr_transmute->value == NULL) > + return -ENOMEM; > + > + xattr_transmute->value_len = TRANS_TRUE_SIZE; > + xattr_transmute->name = XATTR_SMACK_TRANSMUTE; > + } > } > > xattr->value = kstrdup(isp->smk_known, GFP_NOFS); > @@ -3518,20 +3535,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) > * If there is a transmute attribute on the > * directory mark the inode. > */ > - if (isp->smk_flags & SMK_INODE_CHANGED) { > - isp->smk_flags &= ~SMK_INODE_CHANGED; > - rc = __vfs_setxattr(&nop_mnt_idmap, dp, inode, > - XATTR_NAME_SMACKTRANSMUTE, > - TRANS_TRUE, TRANS_TRUE_SIZE, > - 0); > - } else { > - rc = __vfs_getxattr(dp, inode, > - XATTR_NAME_SMACKTRANSMUTE, trattr, > - TRANS_TRUE_SIZE); > - if (rc >= 0 && strncmp(trattr, TRANS_TRUE, > - TRANS_TRUE_SIZE) != 0) > - rc = -EINVAL; > - } > + rc = __vfs_getxattr(dp, inode, > + XATTR_NAME_SMACKTRANSMUTE, trattr, > + TRANS_TRUE_SIZE); > + if (rc >= 0 && strncmp(trattr, TRANS_TRUE, > + TRANS_TRUE_SIZE) != 0) > + rc = -EINVAL; > if (rc >= 0) > transflag = SMK_INODE_TRANSMUTE; > }
On 6/9/2023 9:26 AM, Jarkko Sakkinen wrote: > On Mon Jun 5, 2023 at 11:38 AM EEST, Roberto Sassu wrote: >> On Sat, 2023-06-03 at 21:15 +0200, Roberto Sassu wrote: >>> From: Roberto Sassu <roberto.sassu@huawei.com> >>> >>> With the newly added ability of LSMs to supply multiple xattrs, set >>> SMACK64TRASMUTE in smack_inode_init_security(), instead of d_instantiate(). > > nit: TRANSMUTE > > Sorry, just hit into my eye. I skimmed it because I implemented original > feature :-) Cool! Currently the transmute xattr is defined as: #define XATTR_SMACK_TRANSMUTE "SMACK64TRANSMUTE" so, should be good to say the full xattr name, right? Thanks Roberto
On 6/9/2023 9:35 PM, Mimi Zohar wrote: > Hi Roberto, > > On Sat, 2023-06-03 at 21:15 +0200, Roberto Sassu wrote: >> From: Roberto Sassu <roberto.sassu@huawei.com> >> >> With the newly added ability of LSMs to supply multiple xattrs, set >> SMACK64TRASMUTE in smack_inode_init_security(), instead of d_instantiate(). >> Do it by incrementing SMACK_INODE_INIT_XATTRS to 2 and by calling >> lsm_get_xattr_slot() a second time, if the transmuting conditions are met. >> >> The LSM infrastructure passes all xattrs provided by LSMs to the >> filesystems through the initxattrs() callback, so that filesystems can >> store xattrs in the disk. >> >> After the change, the SMK_INODE_TRANSMUTE inode flag is always set by >> d_instantiate() after fetching SMACK64TRANSMUTE from the disk. Before it >> was done by smack_inode_post_setxattr() as result of the __vfs_setxattr() >> call. >> >> Removing __vfs_setxattr() also prevents invalidating the EVM HMAC, by >> adding a new xattr without checking and updating the existing HMAC. >> >> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > Just a few comments/nits inline. > >> --- >> security/smack/smack.h | 2 +- >> security/smack/smack_lsm.c | 43 +++++++++++++++++++++++--------------- >> 2 files changed, 27 insertions(+), 18 deletions(-) >> >> diff --git a/security/smack/smack.h b/security/smack/smack.h >> index aa15ff56ed6..041688e5a77 100644 >> --- a/security/smack/smack.h >> +++ b/security/smack/smack.h >> @@ -128,7 +128,7 @@ struct task_smack { >> >> #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ >> #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ >> -#define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ >> +#define SMK_INODE_CHANGED 0x04 /* smack was transmuted (unused) */ >> #define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ >> >> /* >> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c >> index a1c30275692..b67d901ee74 100644 >> --- a/security/smack/smack_lsm.c >> +++ b/security/smack/smack_lsm.c >> @@ -52,7 +52,14 @@ >> #define SMK_RECEIVING 1 >> #define SMK_SENDING 2 >> >> -#define SMACK_INODE_INIT_XATTRS 1 >> +/* >> + * Smack uses multiple xattrs. >> + * SMACK64 - for access control, >> + * SMACK64TRANSMUTE - label initialization, >> + * Not saved on files - SMACK64IPIN and SMACK64IPOUT, >> + * Must be set explicitly - SMACK64EXEC and SMACK64MMAP >> + */ >> +#define SMACK_INODE_INIT_XATTRS 2 >> >> #ifdef SMACK_IPV6_PORT_LABELING >> static DEFINE_MUTEX(smack_ipv6_lock); >> @@ -935,7 +942,6 @@ 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); >> @@ -963,6 +969,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, >> if ((tsp->smk_task == tsp->smk_transmuted) || >> (may > 0 && ((may & MAY_TRANSMUTE) != 0) && >> smk_inode_transmutable(dir))) { >> + struct xattr *xattr_transmute; >> + > > Variables should be defined at the beginning of the function. Casey asked to declare the variable in this block. > Is there a reason for beginning the function with "if (xattr) {" > instead "if (!xattr) return 0;". This causes unnecessary indenting. I revisited this part and made few fixes: https://lore.kernel.org/linux-security-module/20230607123612.2791303-1-roberto.sassu@huaweicloud.com/ Patch 3 should address your concern. >> /* >> * The caller of smack_dentry_create_files_as() >> * should have overridden the current cred, so the >> @@ -971,7 +979,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, >> */ >> if (tsp->smk_task != tsp->smk_transmuted) >> isp = dsp; >> - issp->smk_flags |= SMK_INODE_CHANGED; >> + xattr_transmute = lsm_get_xattr_slot(xattrs, xattr_count); >> + if (xattr_transmute) { >> + xattr_transmute->value = kmemdup(TRANS_TRUE, >> + TRANS_TRUE_SIZE, GFP_NOFS); > > script/checkpatch --strict complains here. Thanks, I didn't know about it. It seems that they are more stylistic things. Probably, not worth to respin the patch set just for those (unless you prefer I do it). Thanks Roberto >> + if (xattr_transmute->value == NULL) >> + return -ENOMEM; >> + >> + xattr_transmute->value_len = TRANS_TRUE_SIZE; >> + xattr_transmute->name = XATTR_SMACK_TRANSMUTE; >> + } >> } >> >> xattr->value = kstrdup(isp->smk_known, GFP_NOFS); >> @@ -3518,20 +3535,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) >> * If there is a transmute attribute on the >> * directory mark the inode. >> */ >> - if (isp->smk_flags & SMK_INODE_CHANGED) { >> - isp->smk_flags &= ~SMK_INODE_CHANGED; >> - rc = __vfs_setxattr(&nop_mnt_idmap, dp, inode, >> - XATTR_NAME_SMACKTRANSMUTE, >> - TRANS_TRUE, TRANS_TRUE_SIZE, >> - 0); >> - } else { >> - rc = __vfs_getxattr(dp, inode, >> - XATTR_NAME_SMACKTRANSMUTE, trattr, >> - TRANS_TRUE_SIZE); >> - if (rc >= 0 && strncmp(trattr, TRANS_TRUE, >> - TRANS_TRUE_SIZE) != 0) >> - rc = -EINVAL; >> - } >> + rc = __vfs_getxattr(dp, inode, >> + XATTR_NAME_SMACKTRANSMUTE, trattr, >> + TRANS_TRUE_SIZE); >> + if (rc >= 0 && strncmp(trattr, TRANS_TRUE, >> + TRANS_TRUE_SIZE) != 0) >> + rc = -EINVAL; >> if (rc >= 0) >> transflag = SMK_INODE_TRANSMUTE; >> } > >
On Mon, 2023-06-05 08:38:29 +0000, Roberto Sassu wrote: > > On Sat, 2023-06-03 at 21:15 +0200, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > With the newly added ability of LSMs to supply multiple xattrs, set > > SMACK64TRASMUTE in smack_inode_init_security(), instead of d_instantiate(). > > Do it by incrementing SMACK_INODE_INIT_XATTRS to 2 and by calling > > lsm_get_xattr_slot() a second time, if the transmuting conditions are met. > > > > The LSM infrastructure passes all xattrs provided by LSMs to the > > filesystems through the initxattrs() callback, so that filesystems can > > store xattrs in the disk. > > > > After the change, the SMK_INODE_TRANSMUTE inode flag is always set by > > d_instantiate() after fetching SMACK64TRANSMUTE from the disk. Before it > > was done by smack_inode_post_setxattr() as result of the __vfs_setxattr() > > call. > > > > Removing __vfs_setxattr() also prevents invalidating the EVM HMAC, by > > adding a new xattr without checking and updating the existing HMAC. > > Hi Mengchi > > could you please redo your tests with this patch set applied? > > https://lore.kernel.org/linux-integrity/20230603191518.1397490-1-roberto.sassu@huaweicloud.com/ > > You need: > > https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/log/?h=next > > https://github.com/cschaufler/smack-next/commits/next > > Thanks > > Roberto Sorry for the later reply. It turned out lsm.git repo needs your previous two overlay fs fixes before applying these four patches. With v12 I did not see the issue I reported anymore. Best, Mengchi > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > --- > > security/smack/smack.h | 2 +- > > security/smack/smack_lsm.c | 43 +++++++++++++++++++++++--------------- > > 2 files changed, 27 insertions(+), 18 deletions(-) > > > > diff --git a/security/smack/smack.h b/security/smack/smack.h > > index aa15ff56ed6..041688e5a77 100644 > > --- a/security/smack/smack.h > > +++ b/security/smack/smack.h > > @@ -128,7 +128,7 @@ struct task_smack { > > > > #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ > > #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ > > -#define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ > > +#define SMK_INODE_CHANGED 0x04 /* smack was transmuted (unused) */ > > #define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ > > > > /* > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > > index a1c30275692..b67d901ee74 100644 > > --- a/security/smack/smack_lsm.c > > +++ b/security/smack/smack_lsm.c > > @@ -52,7 +52,14 @@ > > #define SMK_RECEIVING 1 > > #define SMK_SENDING 2 > > > > -#define SMACK_INODE_INIT_XATTRS 1 > > +/* > > + * Smack uses multiple xattrs. > > + * SMACK64 - for access control, > > + * SMACK64TRANSMUTE - label initialization, > > + * Not saved on files - SMACK64IPIN and SMACK64IPOUT, > > + * Must be set explicitly - SMACK64EXEC and SMACK64MMAP > > + */ > > +#define SMACK_INODE_INIT_XATTRS 2 > > > > #ifdef SMACK_IPV6_PORT_LABELING > > static DEFINE_MUTEX(smack_ipv6_lock); > > @@ -935,7 +942,6 @@ 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); > > @@ -963,6 +969,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, > > 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 > > @@ -971,7 +979,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, > > */ > > if (tsp->smk_task != tsp->smk_transmuted) > > isp = dsp; > > - issp->smk_flags |= SMK_INODE_CHANGED; > > + 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; > > + } > > } > > > > xattr->value = kstrdup(isp->smk_known, GFP_NOFS); > > @@ -3518,20 +3535,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) > > * If there is a transmute attribute on the > > * directory mark the inode. > > */ > > - if (isp->smk_flags & SMK_INODE_CHANGED) { > > - isp->smk_flags &= ~SMK_INODE_CHANGED; > > - rc = __vfs_setxattr(&nop_mnt_idmap, dp, inode, > > - XATTR_NAME_SMACKTRANSMUTE, > > - TRANS_TRUE, TRANS_TRUE_SIZE, > > - 0); > > - } else { > > - rc = __vfs_getxattr(dp, inode, > > - XATTR_NAME_SMACKTRANSMUTE, trattr, > > - TRANS_TRUE_SIZE); > > - if (rc >= 0 && strncmp(trattr, TRANS_TRUE, > > - TRANS_TRUE_SIZE) != 0) > > - rc = -EINVAL; > > - } > > + rc = __vfs_getxattr(dp, inode, > > + XATTR_NAME_SMACKTRANSMUTE, trattr, > > + TRANS_TRUE_SIZE); > > + if (rc >= 0 && strncmp(trattr, TRANS_TRUE, > > + TRANS_TRUE_SIZE) != 0) > > + rc = -EINVAL; > > if (rc >= 0) > > transflag = SMK_INODE_TRANSMUTE; > > } > >
diff --git a/security/smack/smack.h b/security/smack/smack.h index aa15ff56ed6..041688e5a77 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -128,7 +128,7 @@ struct task_smack { #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ -#define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ +#define SMK_INODE_CHANGED 0x04 /* smack was transmuted (unused) */ #define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ /* diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index a1c30275692..b67d901ee74 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -52,7 +52,14 @@ #define SMK_RECEIVING 1 #define SMK_SENDING 2 -#define SMACK_INODE_INIT_XATTRS 1 +/* + * Smack uses multiple xattrs. + * SMACK64 - for access control, + * SMACK64TRANSMUTE - label initialization, + * Not saved on files - SMACK64IPIN and SMACK64IPOUT, + * Must be set explicitly - SMACK64EXEC and SMACK64MMAP + */ +#define SMACK_INODE_INIT_XATTRS 2 #ifdef SMACK_IPV6_PORT_LABELING static DEFINE_MUTEX(smack_ipv6_lock); @@ -935,7 +942,6 @@ 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); @@ -963,6 +969,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, 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 @@ -971,7 +979,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, */ if (tsp->smk_task != tsp->smk_transmuted) isp = dsp; - issp->smk_flags |= SMK_INODE_CHANGED; + 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; + } } xattr->value = kstrdup(isp->smk_known, GFP_NOFS); @@ -3518,20 +3535,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) * If there is a transmute attribute on the * directory mark the inode. */ - if (isp->smk_flags & SMK_INODE_CHANGED) { - isp->smk_flags &= ~SMK_INODE_CHANGED; - rc = __vfs_setxattr(&nop_mnt_idmap, dp, inode, - XATTR_NAME_SMACKTRANSMUTE, - TRANS_TRUE, TRANS_TRUE_SIZE, - 0); - } else { - rc = __vfs_getxattr(dp, inode, - XATTR_NAME_SMACKTRANSMUTE, trattr, - TRANS_TRUE_SIZE); - if (rc >= 0 && strncmp(trattr, TRANS_TRUE, - TRANS_TRUE_SIZE) != 0) - rc = -EINVAL; - } + rc = __vfs_getxattr(dp, inode, + XATTR_NAME_SMACKTRANSMUTE, trattr, + TRANS_TRUE_SIZE); + if (rc >= 0 && strncmp(trattr, TRANS_TRUE, + TRANS_TRUE_SIZE) != 0) + rc = -EINVAL; if (rc >= 0) transflag = SMK_INODE_TRANSMUTE; }