* Transmute flag is not inheritted on overlay fs
@ 2023-04-19 0:23 Mengchi Cheng
2023-04-19 2:09 ` Casey Schaufler
0 siblings, 1 reply; 4+ messages in thread
From: Mengchi Cheng @ 2023-04-19 0:23 UTC (permalink / raw)
To: miklos, casey; +Cc: linux-unionfs, linux-security-module, kamatam, yoonjaeh
Hello,
On the overlay ext4 file system, we found that transmute flag is not
inherited by newly created sub-directories. The issue can be recreated on
the newest kernel(6.3.0-rc6) on qemux86-64 with following steps.
/data directory is mounted on /dev/vdb which is a ext4 fs. It is remounted
as an overlay again to upperdir /home/root/data.
# mount -t overlay overlay -o lowerdir=/data,upperdir=/home/root/data,workdir=/home/root/data_work /data
Add a new smack rule and set label and flag to /data directory.
# echo "_ system rwxatl" > /sys/fs/smackfs/load2
# chsmack -a "system" /data
# chsmack -t /data
Create directories under /data.
# mkdir -p /data/dir1/dir2
And then check the smack label of dir1 and dir2.
# chsmack /data/dir1
/data/dir1 access="system"
# chsmack /data/dir1/dir2
/data/dir1/dir2 access="_"
We can see dir1 did not inherit transmute flag from data and dir2 got the
process label.
The transmute xattr of the inode is set inside the smack_d_instantiate
which depends on SMK_INODE_CHANGED bit of isp->smk_flags. But the bit is
not set in the overlay fs mkdir function call chain. So one simple solution
we have is passing inode ptr into smack_dentry_create_files_as and set the
SMK_INODE_CHANGED bit if parent dir is transmuting. Although it looks
reasonable to me and we did not meet any issue in testing, I am not sure if
there is a better solution to it. It will be great, if experts could take
a look.
Thanks,
Mengchi Cheng
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Transmute flag is not inheritted on overlay fs
2023-04-19 0:23 Transmute flag is not inheritted on overlay fs Mengchi Cheng
@ 2023-04-19 2:09 ` Casey Schaufler
2023-04-19 23:18 ` [RFC PATCH] Set SMK_INODE_CHANGED inside smack_dentry_create_files_as Mengchi Cheng
2023-04-19 23:24 ` Transmute flag is not inheritted on overlay fs Mengchi Cheng
0 siblings, 2 replies; 4+ messages in thread
From: Casey Schaufler @ 2023-04-19 2:09 UTC (permalink / raw)
To: Mengchi Cheng, miklos
Cc: linux-unionfs, linux-security-module, kamatam, yoonjaeh, Casey Schaufler
On 4/18/2023 5:23 PM, Mengchi Cheng wrote:
> Hello,
>
> On the overlay ext4 file system, we found that transmute flag is not
> inherited by newly created sub-directories. The issue can be recreated on
> the newest kernel(6.3.0-rc6) on qemux86-64 with following steps.
>
> /data directory is mounted on /dev/vdb which is a ext4 fs. It is remounted
> as an overlay again to upperdir /home/root/data.
> # mount -t overlay overlay -o lowerdir=/data,upperdir=/home/root/data,workdir=/home/root/data_work /data
> Add a new smack rule and set label and flag to /data directory.
> # echo "_ system rwxatl" > /sys/fs/smackfs/load2
> # chsmack -a "system" /data
> # chsmack -t /data
> Create directories under /data.
> # mkdir -p /data/dir1/dir2
> And then check the smack label of dir1 and dir2.
> # chsmack /data/dir1
> /data/dir1 access="system"
> # chsmack /data/dir1/dir2
> /data/dir1/dir2 access="_"
> We can see dir1 did not inherit transmute flag from data and dir2 got the
> process label.
>
> The transmute xattr of the inode is set inside the smack_d_instantiate
> which depends on SMK_INODE_CHANGED bit of isp->smk_flags. But the bit is
> not set in the overlay fs mkdir function call chain. So one simple solution
> we have is passing inode ptr into smack_dentry_create_files_as and set the
> SMK_INODE_CHANGED bit if parent dir is transmuting. Although it looks
> reasonable to me and we did not meet any issue in testing, I am not sure if
> there is a better solution to it. It will be great, if experts could take
> a look.
I will be happy to look at your solution. Please post a patch.
>
>
> Thanks,
> Mengchi Cheng
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH] Set SMK_INODE_CHANGED inside smack_dentry_create_files_as
2023-04-19 2:09 ` Casey Schaufler
@ 2023-04-19 23:18 ` Mengchi Cheng
2023-04-19 23:24 ` Transmute flag is not inheritted on overlay fs Mengchi Cheng
1 sibling, 0 replies; 4+ messages in thread
From: Mengchi Cheng @ 2023-04-19 23:18 UTC (permalink / raw)
To: casey
Cc: kamatam, linux-security-module, linux-unionfs, mengcc, miklos,
yoonjaeh, roberto.sassu
On the overlay fs with smack lsm enabled, new subdir did not inherit
transmute xattr form parent dir.
One solution that can solve it is passing inode into
smack_dentry_create_files_as. And Set SMK_INODE_CHANGED to smak_flags
if directory has transmute xattr.
Reported-by: Ryan Yoon <yoonjaeh@amazon.com>
---
fs/overlayfs/dir.c | 2 +-
include/linux/lsm_hook_defs.h | 2 +-
include/linux/security.h | 4 ++--
security/security.c | 4 ++--
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 8 ++++++--
6 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index fc25fb95d5fc..1b3f7f3a5468 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -598,7 +598,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
override_cred->fsgid = inode->i_gid;
err = security_dentry_create_files_as(dentry,
attr->mode, &dentry->d_name, old_cred,
- override_cred);
+ override_cred, inode);
if (err) {
put_cred(override_cred);
goto out_revert_creds;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 094b76dc7164..96f1fdc21cbc 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -84,7 +84,7 @@ LSM_HOOK(int, -EOPNOTSUPP, dentry_init_security, struct dentry *dentry,
int mode, const struct qstr *name, const char **xattr_name,
void **ctx, u32 *ctxlen)
LSM_HOOK(int, 0, dentry_create_files_as, struct dentry *dentry, int mode,
- struct qstr *name, const struct cred *old, struct cred *new)
+ struct qstr *name, const struct cred *old, struct cred *new, struct inode *inode)
#ifdef CONFIG_SECURITY_PATH
LSM_HOOK(int, 0, path_unlink, const struct path *dir, struct dentry *dentry)
diff --git a/include/linux/security.h b/include/linux/security.h
index 5984d0d550b4..354d68dc69c5 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -325,7 +325,7 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
int security_dentry_create_files_as(struct dentry *dentry, int mode,
struct qstr *name,
const struct cred *old,
- struct cred *new);
+ struct cred *new, struct inode *inode);
int security_path_notify(const struct path *path, u64 mask,
unsigned int obj_type);
int security_inode_alloc(struct inode *inode);
@@ -756,7 +756,7 @@ static inline int security_dentry_init_security(struct dentry *dentry,
static inline int security_dentry_create_files_as(struct dentry *dentry,
int mode, struct qstr *name,
const struct cred *old,
- struct cred *new)
+ struct cred *new, struct inode *inode)
{
return 0;
}
diff --git a/security/security.c b/security/security.c
index cf6cc576736f..0ffe98cc57fe 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1110,10 +1110,10 @@ EXPORT_SYMBOL(security_dentry_init_security);
int security_dentry_create_files_as(struct dentry *dentry, int mode,
struct qstr *name,
- const struct cred *old, struct cred *new)
+ const struct cred *old, struct cred *new, struct inode *inode)
{
return call_int_hook(dentry_create_files_as, 0, dentry, mode,
- name, old, new);
+ name, old, new, inode);
}
EXPORT_SYMBOL(security_dentry_create_files_as);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9a5bdfc21314..2addc513bbb0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2848,7 +2848,7 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
static int selinux_dentry_create_files_as(struct dentry *dentry, int mode,
struct qstr *name,
const struct cred *old,
- struct cred *new)
+ struct cred *new, struct inode *inode)
{
u32 newsid;
int rc;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index cfcbb748da25..e929e3e131c2 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4739,12 +4739,14 @@ static int smack_inode_copy_up_xattr(const char *name)
static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
struct qstr *name,
const struct cred *old,
- struct cred *new)
+ struct cred *new,
+ struct inode *inode)
{
struct task_smack *otsp = smack_cred(old);
struct task_smack *ntsp = smack_cred(new);
struct inode_smack *isp;
int may;
+ struct inode_smack *issp = smack_inode(inode);
/*
* Use the process credential unless all of
@@ -4769,8 +4771,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;
+ issp->smk_flags |= SMK_INODE_CHANGED;
+ }
}
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: Transmute flag is not inheritted on overlay fs
2023-04-19 2:09 ` Casey Schaufler
2023-04-19 23:18 ` [RFC PATCH] Set SMK_INODE_CHANGED inside smack_dentry_create_files_as Mengchi Cheng
@ 2023-04-19 23:24 ` Mengchi Cheng
1 sibling, 0 replies; 4+ messages in thread
From: Mengchi Cheng @ 2023-04-19 23:24 UTC (permalink / raw)
To: casey
Cc: kamatam, linux-security-module, linux-unionfs, mengcc, miklos,
yoonjaeh, roberto.sassu
On Wed, 2023-04-19 02:09:37 +0000, Casey Schaufler wrote:
>
> On 4/18/2023 5:23 PM, Mengchi Cheng wrote:
> > Hello,
> >
> > On the overlay ext4 file system, we found that transmute flag is not
> > inherited by newly created sub-directories. The issue can be recreated on
> > the newest kernel(6.3.0-rc6) on qemux86-64 with following steps.
> >
> > /data directory is mounted on /dev/vdb which is a ext4 fs. It is remounted
> > as an overlay again to upperdir /home/root/data.
> > # mount -t overlay overlay -o lowerdir=/data,upperdir=/home/root/data,workdir=/home/root/data_work /data
> > Add a new smack rule and set label and flag to /data directory.
> > # echo "_ system rwxatl" > /sys/fs/smackfs/load2
> > # chsmack -a "system" /data
> > # chsmack -t /data
> > Create directories under /data.
> > # mkdir -p /data/dir1/dir2
> > And then check the smack label of dir1 and dir2.
> > # chsmack /data/dir1
> > /data/dir1 access="system"
> > # chsmack /data/dir1/dir2
> > /data/dir1/dir2 access="_"
> > We can see dir1 did not inherit transmute flag from data and dir2 got the
> > process label.
> >
> > The transmute xattr of the inode is set inside the smack_d_instantiate
> > which depends on SMK_INODE_CHANGED bit of isp->smk_flags. But the bit is
> > not set in the overlay fs mkdir function call chain. So one simple solution
> > we have is passing inode ptr into smack_dentry_create_files_as and set the
> > SMK_INODE_CHANGED bit if parent dir is transmuting. Although it looks
> > reasonable to me and we did not meet any issue in testing, I am not sure if
> > there is a better solution to it. It will be great, if experts could take
> > a look.
>
> I will be happy to look at your solution. Please post a patch.
>
Sorry, it takes me a while to review and send out the patch.
It contains a few files because it breaks kernel API. But the core is only
in the change of smack_dentry_create_files_as.
If Roberto's patch will work, we can drop it. I posted my concern in that
thread.
https://lore.kernel.org/all/20230419192516.757220-1-mengcc@amazon.com/
> >
> >
> > Thanks,
> > Mengchi Cheng
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-19 23:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-19 0:23 Transmute flag is not inheritted on overlay fs Mengchi Cheng
2023-04-19 2:09 ` Casey Schaufler
2023-04-19 23:18 ` [RFC PATCH] Set SMK_INODE_CHANGED inside smack_dentry_create_files_as Mengchi Cheng
2023-04-19 23:24 ` Transmute flag is not inheritted on overlay fs Mengchi Cheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).