| Message ID | 1649763226-2329-4-git-send-email-xuyang2018.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] idmapped-mounts: Reset errno to zero after detect fs_allow_idmap | expand |
On Tue, Apr 12, 2022 at 07:33:45PM +0800, Yang Xu wrote: > The current_umask() is stripped from the mode directly in the vfs if the > filesystem either doesn't support acls or the filesystem has been > mounted without posic acl support. > > If the filesystem does support acls then current_umask() stripping is > deferred to posix_acl_create(). So when the filesystem calls > posix_acl_create() and there are no acls set or not supported then > current_umask() will be stripped. > > Here we only use umask(S_IXGRP) to check whether inode strip > S_ISGID works correctly. > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > src/idmapped-mounts/idmapped-mounts.c | 505 +++++++++++++++++++++++++- > tests/generic/680 | 26 ++ > tests/generic/680.out | 2 + > 3 files changed, 532 insertions(+), 1 deletion(-) > create mode 100755 tests/generic/680 > create mode 100644 tests/generic/680.out > > diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c > index 02f91558..e6c14586 100644 > --- a/src/idmapped-mounts/idmapped-mounts.c > +++ b/src/idmapped-mounts/idmapped-mounts.c > @@ -14146,6 +14146,494 @@ out: > return fret; > } > > +/* The following tests are concerned with setgid inheritance. These can be > + * filesystem type specific. For xfs, if a new file or directory or node is > + * created within a setgid directory and irix_sgid_inhiert is set then inheritthe > + * setgid bit if the caller is in the group of the directory. > + * > + * The current_umask() is stripped from the mode directly in the vfs if the > + * filesystem either doesn't support acls or the filesystem has been > + * mounted without posic acl support. > + * > + * If the filesystem does support acls then current_umask() stripping is > + * deferred to posix_acl_create(). So when the filesystem calls > + * posix_acl_create() and there are no acls set or not supported then > + * current_umask() will be stripped. > + * > + * Use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly. > + */ > +static int setgid_create_umask(void) > +{ > + int fret = -1; > + int file1_fd = -EBADF; > + int tmpfile_fd = -EBADF; > + pid_t pid; > + bool supported = false; > + char path[PATH_MAX]; > + mode_t mode; > + > + if (!caps_supported()) > + return 0; > + > + if (fchmod(t_dir1_fd, S_IRUSR | > + S_IWUSR | > + S_IRGRP | > + S_IWGRP | > + S_IROTH | > + S_IWOTH | > + S_IXUSR | > + S_IXGRP | > + S_IXOTH | > + S_ISGID), 0) { > + log_stderr("failure: fchmod"); > + goto out; > + } > + > + /* Verify that the setgid bit got raised. */ > + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { > + log_stderr("failure: is_setgid"); > + goto out; > + } > + > + supported = openat_tmpfile_supported(t_dir1_fd); > + > + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode > + * whether has group execute or search permission. > + */ > + umask(S_IXGRP); > + mode = umask(S_IXGRP); > + if (!(mode & S_IXGRP)) > + die("failure: umask"); > + > + pid = fork(); > + if (pid < 0) { > + log_stderr("failure: fork"); > + goto out; > + } > + if (pid == 0) { > + if (!switch_ids(0, 10000)) > + die("failure: switch_ids"); > + > + if (!caps_down_fsetid()) > + die("failure: caps_down_fsetid"); > + > + /* create regular file via open() */ > + file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); > + if (file1_fd < 0) > + die("failure: create"); > + > + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid > + * bit needs to be stripped. > + */ > + if (is_setgid(t_dir1_fd, FILE1, 0)) > + die("failure: is_setgid"); This test is wrong. Specifically, it is a false positive. I have explained this in more detail on v2 of this patchset. You want to test that umask(S_IXGRP) + setgid inheritance work together correctly. First, we need to establish what that means because from your patch series it at least seems to me as you're not completely clear about what you want to test just yet. A requested setgid bit for a non-directory object is only considered for stripping if S_IXGRP is simultaneously requested. In other words, both S_SISGID and S_IXGRP must be requested for the new file in order for additional checks such as CAP_FSETID to become relevant. We need to distinguish two cases afaict: 1. The filesystem does support ACLs and has an applicable ACL ------------------------------------------------------------- umask(S_IXGRP) is not used by the kernel and thus S_IXGRP is not stripped (unless the ACL does make it so) and so when e.g. inode_init_owner() is called we do not expect the file to inherit the setgid bit. This is what your test above is handling correctly. But it is a false positive because what you're trying to test is the behavior of umask(S_IXGRP) but it is made irrelevant by ACL support of the underlying filesystem. 2. The filesystem does not support ACLs, has been mounted without ----------------------------------------------------------------- support for it, or has no applicable ACL ---------------------------------------- umask(S_IXGRP) is used by the kernel and will be stripped from the mode. So when inode_init_owner() is called we expect the file to inherit the setgid bit. This means the test for this case needs to be: file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); if (file1_fd < 0) die("failure: create"); /* * S_IXGRP will have been removed due to umask(S_IXGRP) so we expect the * new file to be S_ISGID. */ if (!is_setgid(t_dir1_fd, FILE1, 0)) die("failure: is_setgid"); And additionally you might want to also add a new helper is_ixgrp() to add an additional check: /* * S_IXGRP will have been removed due to umask(S_IXGRP) so we expect the * new file to not be S_IXGRP. */ if (is_ixgrp(t_dir1_fd, FILE1, 0)) die("failure: is_ixgrp"); --- So for the umask() tests you need to first check whether the underlying filesystem does support ACLs and remember that somewhere. If it does support ACLs, then you should first remove any ACLs set on the directories you're performing the tests on. Then you can run your umask() tests.
on 2022/4/13 16:59, Christian Brauner wrote: > On Tue, Apr 12, 2022 at 07:33:45PM +0800, Yang Xu wrote: >> The current_umask() is stripped from the mode directly in the vfs if the >> filesystem either doesn't support acls or the filesystem has been >> mounted without posic acl support. >> >> If the filesystem does support acls then current_umask() stripping is >> deferred to posix_acl_create(). So when the filesystem calls >> posix_acl_create() and there are no acls set or not supported then >> current_umask() will be stripped. >> >> Here we only use umask(S_IXGRP) to check whether inode strip >> S_ISGID works correctly. >> >> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com> >> --- >> src/idmapped-mounts/idmapped-mounts.c | 505 +++++++++++++++++++++++++- >> tests/generic/680 | 26 ++ >> tests/generic/680.out | 2 + >> 3 files changed, 532 insertions(+), 1 deletion(-) >> create mode 100755 tests/generic/680 >> create mode 100644 tests/generic/680.out >> >> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c >> index 02f91558..e6c14586 100644 >> --- a/src/idmapped-mounts/idmapped-mounts.c >> +++ b/src/idmapped-mounts/idmapped-mounts.c >> @@ -14146,6 +14146,494 @@ out: >> return fret; >> } >> >> +/* The following tests are concerned with setgid inheritance. These can be >> + * filesystem type specific. For xfs, if a new file or directory or node is >> + * created within a setgid directory and irix_sgid_inhiert is set then inheritthe >> + * setgid bit if the caller is in the group of the directory. >> + * >> + * The current_umask() is stripped from the mode directly in the vfs if the >> + * filesystem either doesn't support acls or the filesystem has been >> + * mounted without posic acl support. >> + * >> + * If the filesystem does support acls then current_umask() stripping is >> + * deferred to posix_acl_create(). So when the filesystem calls >> + * posix_acl_create() and there are no acls set or not supported then >> + * current_umask() will be stripped. >> + * >> + * Use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly. >> + */ >> +static int setgid_create_umask(void) >> +{ >> + int fret = -1; >> + int file1_fd = -EBADF; >> + int tmpfile_fd = -EBADF; >> + pid_t pid; >> + bool supported = false; >> + char path[PATH_MAX]; >> + mode_t mode; >> + >> + if (!caps_supported()) >> + return 0; >> + >> + if (fchmod(t_dir1_fd, S_IRUSR | >> + S_IWUSR | >> + S_IRGRP | >> + S_IWGRP | >> + S_IROTH | >> + S_IWOTH | >> + S_IXUSR | >> + S_IXGRP | >> + S_IXOTH | >> + S_ISGID), 0) { >> + log_stderr("failure: fchmod"); >> + goto out; >> + } >> + >> + /* Verify that the setgid bit got raised. */ >> + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { >> + log_stderr("failure: is_setgid"); >> + goto out; >> + } >> + >> + supported = openat_tmpfile_supported(t_dir1_fd); >> + >> + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode >> + * whether has group execute or search permission. >> + */ >> + umask(S_IXGRP); >> + mode = umask(S_IXGRP); >> + if (!(mode& S_IXGRP)) >> + die("failure: umask"); >> + >> + pid = fork(); >> + if (pid< 0) { >> + log_stderr("failure: fork"); >> + goto out; >> + } >> + if (pid == 0) { >> + if (!switch_ids(0, 10000)) >> + die("failure: switch_ids"); >> + >> + if (!caps_down_fsetid()) >> + die("failure: caps_down_fsetid"); >> + >> + /* create regular file via open() */ >> + file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); >> + if (file1_fd< 0) >> + die("failure: create"); >> + >> + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid >> + * bit needs to be stripped. >> + */ >> + if (is_setgid(t_dir1_fd, FILE1, 0)) >> + die("failure: is_setgid"); > > This test is wrong. Specifically, it is a false positive. I have > explained this in more detail on v2 of this patchset. > > You want to test that umask(S_IXGRP) + setgid inheritance work together > correctly. First, we need to establish what that means because from your > patch series it at least seems to me as you're not completely clear > about what you want to test just yet. > > A requested setgid bit for a non-directory object is only considered for > stripping if S_IXGRP is simultaneously requested. In other words, both > S_SISGID and S_IXGRP must be requested for the new file in order for > additional checks such as CAP_FSETID to become relevant. Yes, only keep S_IXGRP, then we can run into the next judgement for group and cap_fsetid. > > We need to distinguish two cases afaict: > > 1. The filesystem does support ACLs and has an applicable ACL > ------------------------------------------------------------- > > umask(S_IXGRP) is not used by the kernel and thus S_IXGRP is not > stripped (unless the ACL does make it so) and so when e.g. > inode_init_owner() is called we do not expect the file to inherit the > setgid bit. > > This is what your test above is handling correctly. But it is a false > positive because what you're trying to test is the behavior of > umask(S_IXGRP) but it is made irrelevant by ACL support of the > underlying filesystem. I test this situation in the next patch as below: umask(S_IXGRP); snprintf(t_buf, sizeof(t_buf), "setfacl -d -m u::rwx,g::rwx,o::rwx,m:rw %s/%s", t_mountpoint, T_DIR1) and umask(S_IXGRP); snprintf(t_buf, sizeof(t_buf), "setfacl -d -m u::rwx,g::rw,o::rwx, %s/%s", t_mountpoint, T_DIR1 > > 2. The filesystem does not support ACLs, has been mounted without > ----------------------------------------------------------------- > support for it, or has no applicable ACL > ---------------------------------------- > > umask(S_IXGRP) is used by the kernel and will be stripped from the mode. > So when inode_init_owner() is called we expect the file to inherit the > setgid bit. This is why my kernel patch put strip setgid code into vfs. xfs will inherit the setgid bit but ext4 not because the new_inode function and posix_acl_create function order(S_IXGRP mode has been stripped before pass to inode_init_owner). > > This means the test for this case needs to be: > > file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); > if (file1_fd< 0) > die("failure: create"); > > /* > * S_IXGRP will have been removed due to umask(S_IXGRP) so we expect the > * new file to be S_ISGID. No No No, see the kernel patch url https://patchwork.kernel.org/project/linux-fsdevel/patch/1648461389-2225-2-git-send-email-xuyang2018.jy@fujitsu.com/ > */ > if (!is_setgid(t_dir1_fd, FILE1, 0)) > die("failure: is_setgid"); > > And additionally you might want to also add a new helper is_ixgrp() to > add an additional check: > > /* > * S_IXGRP will have been removed due to umask(S_IXGRP) so we expect the > * new file to not be S_IXGRP. > */ > if (is_ixgrp(t_dir1_fd, FILE1, 0)) > die("failure: is_ixgrp"); This sound reasonable. > > > --- > > So for the umask() tests you need to first check whether the underlying > filesystem does support ACLs and remember that somewhere. If it does > support ACLs, then you should first remove any ACLs set on the > directories you're performing the tests on. Then you can run your > umask() tests. Yes, I can just add a remove step in the beginning. ps: When I writing my v2 kernel patch, I found I still miss the GRPID testcase in fstests. Best Regrads Yang Xu
On Wed, Apr 13, 2022 at 09:45:11AM +0000, xuyang2018.jy@fujitsu.com wrote: > on 2022/4/13 16:59, Christian Brauner wrote: > > On Tue, Apr 12, 2022 at 07:33:45PM +0800, Yang Xu wrote: > >> The current_umask() is stripped from the mode directly in the vfs if the > >> filesystem either doesn't support acls or the filesystem has been > >> mounted without posic acl support. > >> > >> If the filesystem does support acls then current_umask() stripping is > >> deferred to posix_acl_create(). So when the filesystem calls > >> posix_acl_create() and there are no acls set or not supported then > >> current_umask() will be stripped. > >> > >> Here we only use umask(S_IXGRP) to check whether inode strip > >> S_ISGID works correctly. > >> > >> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com> > >> --- > >> src/idmapped-mounts/idmapped-mounts.c | 505 +++++++++++++++++++++++++- > >> tests/generic/680 | 26 ++ > >> tests/generic/680.out | 2 + > >> 3 files changed, 532 insertions(+), 1 deletion(-) > >> create mode 100755 tests/generic/680 > >> create mode 100644 tests/generic/680.out > >> > >> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c > >> index 02f91558..e6c14586 100644 > >> --- a/src/idmapped-mounts/idmapped-mounts.c > >> +++ b/src/idmapped-mounts/idmapped-mounts.c > >> @@ -14146,6 +14146,494 @@ out: > >> return fret; > >> } > >> > >> +/* The following tests are concerned with setgid inheritance. These can be > >> + * filesystem type specific. For xfs, if a new file or directory or node is > >> + * created within a setgid directory and irix_sgid_inhiert is set then inheritthe > >> + * setgid bit if the caller is in the group of the directory. > >> + * > >> + * The current_umask() is stripped from the mode directly in the vfs if the > >> + * filesystem either doesn't support acls or the filesystem has been > >> + * mounted without posic acl support. > >> + * > >> + * If the filesystem does support acls then current_umask() stripping is > >> + * deferred to posix_acl_create(). So when the filesystem calls > >> + * posix_acl_create() and there are no acls set or not supported then > >> + * current_umask() will be stripped. > >> + * > >> + * Use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly. > >> + */ > >> +static int setgid_create_umask(void) > >> +{ > >> + int fret = -1; > >> + int file1_fd = -EBADF; > >> + int tmpfile_fd = -EBADF; > >> + pid_t pid; > >> + bool supported = false; > >> + char path[PATH_MAX]; > >> + mode_t mode; > >> + > >> + if (!caps_supported()) > >> + return 0; > >> + > >> + if (fchmod(t_dir1_fd, S_IRUSR | > >> + S_IWUSR | > >> + S_IRGRP | > >> + S_IWGRP | > >> + S_IROTH | > >> + S_IWOTH | > >> + S_IXUSR | > >> + S_IXGRP | > >> + S_IXOTH | > >> + S_ISGID), 0) { > >> + log_stderr("failure: fchmod"); > >> + goto out; > >> + } > >> + > >> + /* Verify that the setgid bit got raised. */ > >> + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { > >> + log_stderr("failure: is_setgid"); > >> + goto out; > >> + } > >> + > >> + supported = openat_tmpfile_supported(t_dir1_fd); > >> + > >> + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode > >> + * whether has group execute or search permission. > >> + */ > >> + umask(S_IXGRP); > >> + mode = umask(S_IXGRP); > >> + if (!(mode& S_IXGRP)) > >> + die("failure: umask"); > >> + > >> + pid = fork(); > >> + if (pid< 0) { > >> + log_stderr("failure: fork"); > >> + goto out; > >> + } > >> + if (pid == 0) { > >> + if (!switch_ids(0, 10000)) > >> + die("failure: switch_ids"); > >> + > >> + if (!caps_down_fsetid()) > >> + die("failure: caps_down_fsetid"); > >> + > >> + /* create regular file via open() */ > >> + file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); > >> + if (file1_fd< 0) > >> + die("failure: create"); > >> + > >> + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid > >> + * bit needs to be stripped. > >> + */ > >> + if (is_setgid(t_dir1_fd, FILE1, 0)) > >> + die("failure: is_setgid"); > > > > This test is wrong. Specifically, it is a false positive. I have > > explained this in more detail on v2 of this patchset. > > > > You want to test that umask(S_IXGRP) + setgid inheritance work together > > correctly. First, we need to establish what that means because from your > > patch series it at least seems to me as you're not completely clear > > about what you want to test just yet. > > > > A requested setgid bit for a non-directory object is only considered for > > stripping if S_IXGRP is simultaneously requested. In other words, both > > S_SISGID and S_IXGRP must be requested for the new file in order for > > additional checks such as CAP_FSETID to become relevant. > Yes, only keep S_IXGRP, then we can run into the next judgement for > group and cap_fsetid. > > > > We need to distinguish two cases afaict: > > > > 1. The filesystem does support ACLs and has an applicable ACL > > ------------------------------------------------------------- > > > > umask(S_IXGRP) is not used by the kernel and thus S_IXGRP is not > > stripped (unless the ACL does make it so) and so when e.g. > > inode_init_owner() is called we do not expect the file to inherit the > > setgid bit. > > > > This is what your test above is handling correctly. But it is a false > > positive because what you're trying to test is the behavior of > > umask(S_IXGRP) but it is made irrelevant by ACL support of the > > underlying filesystem. > I test this situation in the next patch as below: > umask(S_IXGRP); > snprintf(t_buf, sizeof(t_buf), "setfacl -d -m u::rwx,g::rwx,o::rwx,m:rw > %s/%s", t_mountpoint, T_DIR1) > > and > umask(S_IXGRP); > snprintf(t_buf, sizeof(t_buf), "setfacl -d -m u::rwx,g::rw,o::rwx, > %s/%s", t_mountpoint, T_DIR1 > > > > > 2. The filesystem does not support ACLs, has been mounted without > > ----------------------------------------------------------------- > > support for it, or has no applicable ACL > > ---------------------------------------- > > > > umask(S_IXGRP) is used by the kernel and will be stripped from the mode. > > So when inode_init_owner() is called we expect the file to inherit the > > setgid bit. > This is why my kernel patch put strip setgid code into vfs. > xfs will inherit the setgid bit but ext4 not because the new_inode > function and posix_acl_create function order(S_IXGRP mode has been > stripped before pass to inode_init_owner). > > > > This means the test for this case needs to be: > > > > file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); > > if (file1_fd< 0) > > die("failure: create"); > > > > /* > > * S_IXGRP will have been removed due to umask(S_IXGRP) so we expect the > > * new file to be S_ISGID. > No No No, see the kernel patch url > https://patchwork.kernel.org/project/linux-fsdevel/patch/1648461389-2225-2-git-send-email-xuyang2018.jy@fujitsu.com/ Ok, so your testing patches are premised on your kernel patchset. That wasn't clear to me. The kernel patchset removes the setgid bit _before_ the umask is applied which is why you're expecting this file to not be setgid because you also dropped CAP_FSETID and you'r not in the group of the directory. (Ok, let's defer that discussion to the updated patchset.)
on 2022/4/13 17:59, Christian Brauner write: > On Wed, Apr 13, 2022 at 09:45:11AM +0000, xuyang2018.jy@fujitsu.com wrote: >> on 2022/4/13 16:59, Christian Brauner wrote: >>> On Tue, Apr 12, 2022 at 07:33:45PM +0800, Yang Xu wrote: >>>> The current_umask() is stripped from the mode directly in the vfs if the >>>> filesystem either doesn't support acls or the filesystem has been >>>> mounted without posic acl support. >>>> >>>> If the filesystem does support acls then current_umask() stripping is >>>> deferred to posix_acl_create(). So when the filesystem calls >>>> posix_acl_create() and there are no acls set or not supported then >>>> current_umask() will be stripped. >>>> >>>> Here we only use umask(S_IXGRP) to check whether inode strip >>>> S_ISGID works correctly. >>>> >>>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com> >>>> --- >>>> src/idmapped-mounts/idmapped-mounts.c | 505 +++++++++++++++++++++++++- >>>> tests/generic/680 | 26 ++ >>>> tests/generic/680.out | 2 + >>>> 3 files changed, 532 insertions(+), 1 deletion(-) >>>> create mode 100755 tests/generic/680 >>>> create mode 100644 tests/generic/680.out >>>> >>>> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c >>>> index 02f91558..e6c14586 100644 >>>> --- a/src/idmapped-mounts/idmapped-mounts.c >>>> +++ b/src/idmapped-mounts/idmapped-mounts.c >>>> @@ -14146,6 +14146,494 @@ out: >>>> return fret; >>>> } >>>> >>>> +/* The following tests are concerned with setgid inheritance. These can be >>>> + * filesystem type specific. For xfs, if a new file or directory or node is >>>> + * created within a setgid directory and irix_sgid_inhiert is set then inheritthe >>>> + * setgid bit if the caller is in the group of the directory. >>>> + * >>>> + * The current_umask() is stripped from the mode directly in the vfs if the >>>> + * filesystem either doesn't support acls or the filesystem has been >>>> + * mounted without posic acl support. >>>> + * >>>> + * If the filesystem does support acls then current_umask() stripping is >>>> + * deferred to posix_acl_create(). So when the filesystem calls >>>> + * posix_acl_create() and there are no acls set or not supported then >>>> + * current_umask() will be stripped. >>>> + * >>>> + * Use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly. >>>> + */ >>>> +static int setgid_create_umask(void) >>>> +{ >>>> + int fret = -1; >>>> + int file1_fd = -EBADF; >>>> + int tmpfile_fd = -EBADF; >>>> + pid_t pid; >>>> + bool supported = false; >>>> + char path[PATH_MAX]; >>>> + mode_t mode; >>>> + >>>> + if (!caps_supported()) >>>> + return 0; >>>> + >>>> + if (fchmod(t_dir1_fd, S_IRUSR | >>>> + S_IWUSR | >>>> + S_IRGRP | >>>> + S_IWGRP | >>>> + S_IROTH | >>>> + S_IWOTH | >>>> + S_IXUSR | >>>> + S_IXGRP | >>>> + S_IXOTH | >>>> + S_ISGID), 0) { >>>> + log_stderr("failure: fchmod"); >>>> + goto out; >>>> + } >>>> + >>>> + /* Verify that the setgid bit got raised. */ >>>> + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { >>>> + log_stderr("failure: is_setgid"); >>>> + goto out; >>>> + } >>>> + >>>> + supported = openat_tmpfile_supported(t_dir1_fd); >>>> + >>>> + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode >>>> + * whether has group execute or search permission. >>>> + */ >>>> + umask(S_IXGRP); >>>> + mode = umask(S_IXGRP); >>>> + if (!(mode& S_IXGRP)) >>>> + die("failure: umask"); >>>> + >>>> + pid = fork(); >>>> + if (pid< 0) { >>>> + log_stderr("failure: fork"); >>>> + goto out; >>>> + } >>>> + if (pid == 0) { >>>> + if (!switch_ids(0, 10000)) >>>> + die("failure: switch_ids"); >>>> + >>>> + if (!caps_down_fsetid()) >>>> + die("failure: caps_down_fsetid"); >>>> + >>>> + /* create regular file via open() */ >>>> + file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); >>>> + if (file1_fd< 0) >>>> + die("failure: create"); >>>> + >>>> + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid >>>> + * bit needs to be stripped. >>>> + */ >>>> + if (is_setgid(t_dir1_fd, FILE1, 0)) >>>> + die("failure: is_setgid"); >>> >>> This test is wrong. Specifically, it is a false positive. I have >>> explained this in more detail on v2 of this patchset. >>> >>> You want to test that umask(S_IXGRP) + setgid inheritance work together >>> correctly. First, we need to establish what that means because from your >>> patch series it at least seems to me as you're not completely clear >>> about what you want to test just yet. >>> >>> A requested setgid bit for a non-directory object is only considered for >>> stripping if S_IXGRP is simultaneously requested. In other words, both >>> S_SISGID and S_IXGRP must be requested for the new file in order for >>> additional checks such as CAP_FSETID to become relevant. >> Yes, only keep S_IXGRP, then we can run into the next judgement for >> group and cap_fsetid. >>> >>> We need to distinguish two cases afaict: >>> >>> 1. The filesystem does support ACLs and has an applicable ACL >>> ------------------------------------------------------------- >>> >>> umask(S_IXGRP) is not used by the kernel and thus S_IXGRP is not >>> stripped (unless the ACL does make it so) and so when e.g. >>> inode_init_owner() is called we do not expect the file to inherit the >>> setgid bit. >>> >>> This is what your test above is handling correctly. But it is a false >>> positive because what you're trying to test is the behavior of >>> umask(S_IXGRP) but it is made irrelevant by ACL support of the >>> underlying filesystem. >> I test this situation in the next patch as below: >> umask(S_IXGRP); >> snprintf(t_buf, sizeof(t_buf), "setfacl -d -m u::rwx,g::rwx,o::rwx,m:rw >> %s/%s", t_mountpoint, T_DIR1) >> >> and >> umask(S_IXGRP); >> snprintf(t_buf, sizeof(t_buf), "setfacl -d -m u::rwx,g::rw,o::rwx, >> %s/%s", t_mountpoint, T_DIR1 >> >>> >>> 2. The filesystem does not support ACLs, has been mounted without >>> ----------------------------------------------------------------- >>> support for it, or has no applicable ACL >>> ---------------------------------------- >>> >>> umask(S_IXGRP) is used by the kernel and will be stripped from the mode. >>> So when inode_init_owner() is called we expect the file to inherit the >>> setgid bit. >> This is why my kernel patch put strip setgid code into vfs. >> xfs will inherit the setgid bit but ext4 not because the new_inode >> function and posix_acl_create function order(S_IXGRP mode has been >> stripped before pass to inode_init_owner). >>> >>> This means the test for this case needs to be: >>> >>> file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); >>> if (file1_fd< 0) >>> die("failure: create"); >>> >>> /* >>> * S_IXGRP will have been removed due to umask(S_IXGRP) so we expect the >>> * new file to be S_ISGID. >> No No No, see the kernel patch url >> https://patchwork.kernel.org/project/linux-fsdevel/patch/1648461389-2225-2-git-send-email-xuyang2018.jy@fujitsu.com/ > > Ok, so your testing patches are premised on your kernel patchset. That > wasn't clear to me. > Because of Darrick's request when reviewing this kernel patchset, so then I begin to refactor setgid_create code in fstets... I should have added this in this patch' commit message, sorry. Will add it in v4. > The kernel patchset removes the setgid bit _before_ the umask is applied > which is why you're expecting this file to not be setgid because you > also dropped CAP_FSETID and you'r not in the group of the directory. Yes. > > (Ok, let's defer that discussion to the updated patchset.) I am rewritting the kernel patch commit message to clarify why put this strip code into vfs is safer, but I still have several filesystem code not to see. I will send a v2 kernel patchset tomorrow. Best Regards Yang Xu
diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c index 02f91558..e6c14586 100644 --- a/src/idmapped-mounts/idmapped-mounts.c +++ b/src/idmapped-mounts/idmapped-mounts.c @@ -14146,6 +14146,494 @@ out: return fret; } +/* The following tests are concerned with setgid inheritance. These can be + * filesystem type specific. For xfs, if a new file or directory or node is + * created within a setgid directory and irix_sgid_inhiert is set then inheritthe + * setgid bit if the caller is in the group of the directory. + * + * The current_umask() is stripped from the mode directly in the vfs if the + * filesystem either doesn't support acls or the filesystem has been + * mounted without posic acl support. + * + * If the filesystem does support acls then current_umask() stripping is + * deferred to posix_acl_create(). So when the filesystem calls + * posix_acl_create() and there are no acls set or not supported then + * current_umask() will be stripped. + * + * Use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly. + */ +static int setgid_create_umask(void) +{ + int fret = -1; + int file1_fd = -EBADF; + int tmpfile_fd = -EBADF; + pid_t pid; + bool supported = false; + char path[PATH_MAX]; + mode_t mode; + + if (!caps_supported()) + return 0; + + if (fchmod(t_dir1_fd, S_IRUSR | + S_IWUSR | + S_IRGRP | + S_IWGRP | + S_IROTH | + S_IWOTH | + S_IXUSR | + S_IXGRP | + S_IXOTH | + S_ISGID), 0) { + log_stderr("failure: fchmod"); + goto out; + } + + /* Verify that the setgid bit got raised. */ + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { + log_stderr("failure: is_setgid"); + goto out; + } + + supported = openat_tmpfile_supported(t_dir1_fd); + + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode + * whether has group execute or search permission. + */ + umask(S_IXGRP); + mode = umask(S_IXGRP); + if (!(mode & S_IXGRP)) + die("failure: umask"); + + pid = fork(); + if (pid < 0) { + log_stderr("failure: fork"); + goto out; + } + if (pid == 0) { + if (!switch_ids(0, 10000)) + die("failure: switch_ids"); + + if (!caps_down_fsetid()) + die("failure: caps_down_fsetid"); + + /* create regular file via open() */ + file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); + if (file1_fd < 0) + die("failure: create"); + + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid + * bit needs to be stripped. + */ + if (is_setgid(t_dir1_fd, FILE1, 0)) + die("failure: is_setgid"); + + /* create directory */ + if (mkdirat(t_dir1_fd, DIR1, 0000)) + die("failure: create"); + + if (xfs_irix_sgid_inherit_enabled()) { + /* We're not in_group_p(). */ + if (is_setgid(t_dir1_fd, DIR1, 0)) + die("failure: is_setgid"); + } else { + /* Directories always inherit the setgid bit. */ + if (!is_setgid(t_dir1_fd, DIR1, 0)) + die("failure: is_setgid"); + } + + /* create a special file via mknodat() vfs_create */ + if (mknodat(t_dir1_fd, FILE2, S_IFREG | S_ISGID | S_IXGRP, 0)) + die("failure: mknodat"); + + if (is_setgid(t_dir1_fd, FILE2, 0)) + die("failure: is_setgid"); + + /* create a character device via mknodat() vfs_mknod */ + if (mknodat(t_dir1_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, makedev(5, 1))) + die("failure: mknodat"); + + if (is_setgid(t_dir1_fd, CHRDEV1, 0)) + die("failure: is_setgid"); + + if (unlinkat(t_dir1_fd, FILE1, 0)) + die("failure: delete"); + + if (unlinkat(t_dir1_fd, DIR1, AT_REMOVEDIR)) + die("failure: delete"); + + if (unlinkat(t_dir1_fd, FILE2, 0)) + die("failure: delete"); + + if (unlinkat(t_dir1_fd, CHRDEV1, 0)) + die("failure: delete"); + + /* create tmpfile via filesystem tmpfile api */ + if (supported) { + tmpfile_fd = openat(t_dir1_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID); + if (tmpfile_fd < 0) + die("failure: create"); + /* link the temporary file into the filesystem, making it permanent */ + snprintf(path, PATH_MAX, "/proc/self/fd/%d", tmpfile_fd); + if (linkat(AT_FDCWD, path, t_dir1_fd, FILE3, AT_SYMLINK_FOLLOW)) + die("failure: linkat"); + if (close(tmpfile_fd)) + die("failure: close"); + if (is_setgid(t_dir1_fd, FILE3, 0)) + die("failure: is_setgid"); + if (unlinkat(t_dir1_fd, FILE3, 0)) + die("failure: delete"); + } + + exit(EXIT_SUCCESS); + } + if (wait_for_pid(pid)) + goto out; + + fret = 0; + log_debug("Ran test"); +out: + safe_close(file1_fd); + return fret; +} + +static int setgid_create_umask_idmapped(void) +{ + int fret = -1; + int file1_fd = -EBADF, open_tree_fd = -EBADF; + struct mount_attr attr = { + .attr_set = MOUNT_ATTR_IDMAP, + }; + pid_t pid; + int tmpfile_fd = -EBADF; + bool supported = false; + char path[PATH_MAX]; + mode_t mode; + + if (!caps_supported()) + return 0; + + if (fchmod(t_dir1_fd, S_IRUSR | + S_IWUSR | + S_IRGRP | + S_IWGRP | + S_IROTH | + S_IWOTH | + S_IXUSR | + S_IXGRP | + S_IXOTH | + S_ISGID), 0) { + log_stderr("failure: fchmod"); + goto out; + } + + /* Verify that the sid bits got raised. */ + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { + log_stderr("failure: is_setgid"); + goto out; + } + + /* Changing mount properties on a detached mount. */ + attr.userns_fd = get_userns_fd(0, 10000, 10000); + if (attr.userns_fd < 0) { + log_stderr("failure: get_userns_fd"); + goto out; + } + + open_tree_fd = sys_open_tree(t_dir1_fd, "", + AT_EMPTY_PATH | + AT_NO_AUTOMOUNT | + AT_SYMLINK_NOFOLLOW | + OPEN_TREE_CLOEXEC | + OPEN_TREE_CLONE); + if (open_tree_fd < 0) { + log_stderr("failure: sys_open_tree"); + goto out; + } + + if (sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr))) { + log_stderr("failure: sys_mount_setattr"); + goto out; + } + + supported = openat_tmpfile_supported(open_tree_fd); + + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode + * whether has group execute or search permission. + */ + umask(S_IXGRP); + mode = umask(S_IXGRP); + if (!(mode & S_IXGRP)) + die("failure: umask"); + + pid = fork(); + if (pid < 0) { + log_stderr("failure: fork"); + goto out; + } + if (pid == 0) { + if (!switch_ids(10000, 11000)) + die("failure: switch fsids"); + + /* create regular file via open() */ + file1_fd = openat(open_tree_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); + if (file1_fd < 0) + die("failure: create"); + + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid + * bit needs to be stripped. + */ + if (is_setgid(open_tree_fd, FILE1, 0)) + die("failure: is_setgid"); + + /* create directory */ + if (mkdirat(open_tree_fd, DIR1, 0000)) + die("failure: create"); + + if (xfs_irix_sgid_inherit_enabled()) { + /* We're not in_group_p(). */ + if (is_setgid(open_tree_fd, DIR1, 0)) + die("failure: is_setgid"); + } else { + /* Directories always inherit the setgid bit. */ + if (!is_setgid(open_tree_fd, DIR1, 0)) + die("failure: is_setgid"); + } + + /* create a special file via mknodat() vfs_create */ + if (mknodat(open_tree_fd, FILE2, S_IFREG | S_ISGID | S_IXGRP, 0)) + die("failure: mknodat"); + + if (is_setgid(open_tree_fd, FILE2, 0)) + die("failure: is_setgid"); + + /* create a whiteout device via mknodat() vfs_mknod */ + if (mknodat(open_tree_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, 0)) + die("failure: mknodat"); + + if (is_setgid(open_tree_fd, CHRDEV1, 0)) + die("failure: is_setgid"); + + if (unlinkat(open_tree_fd, FILE1, 0)) + die("failure: delete"); + + if (unlinkat(open_tree_fd, DIR1, AT_REMOVEDIR)) + die("failure: delete"); + + if (unlinkat(open_tree_fd, FILE2, 0)) + die("failure: delete"); + + if (unlinkat(open_tree_fd, CHRDEV1, 0)) + die("failure: delete"); + + /* create tmpfile via filesystem tmpfile api */ + if (supported) { + tmpfile_fd = openat(open_tree_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID); + if (tmpfile_fd < 0) + die("failure: create"); + /* link the temporary file into the filesystem, making it permanent */ + snprintf(path, PATH_MAX, "/proc/self/fd/%d", tmpfile_fd); + if (linkat(AT_FDCWD, path, open_tree_fd, FILE3, AT_SYMLINK_FOLLOW)) + die("failure: linkat"); + if (close(tmpfile_fd)) + die("failure: close"); + if (is_setgid(open_tree_fd, FILE3, 0)) + die("failure: is_setgid"); + if (unlinkat(open_tree_fd, FILE3, 0)) + die("failure: delete"); + } + + exit(EXIT_SUCCESS); + } + if (wait_for_pid(pid)) + goto out; + + fret = 0; + log_debug("Ran test"); +out: + safe_close(attr.userns_fd); + safe_close(file1_fd); + safe_close(open_tree_fd); + + return fret; +} + +static int setgid_create_umask_idmapped_in_userns(void) +{ + int fret = -1; + int file1_fd = -EBADF, open_tree_fd = -EBADF; + struct mount_attr attr = { + .attr_set = MOUNT_ATTR_IDMAP, + }; + pid_t pid; + int tmpfile_fd = -EBADF; + bool supported = false; + char path[PATH_MAX]; + mode_t mode; + + if (!caps_supported()) + return 0; + + if (fchmod(t_dir1_fd, S_IRUSR | + S_IWUSR | + S_IRGRP | + S_IWGRP | + S_IROTH | + S_IWOTH | + S_IXUSR | + S_IXGRP | + S_IXOTH | + S_ISGID), 0) { + log_stderr("failure: fchmod"); + goto out; + } + + /* Verify that the sid bits got raised. */ + if (!is_setgid(t_dir1_fd, "", AT_EMPTY_PATH)) { + log_stderr("failure: is_setgid"); + goto out; + } + + /* Changing mount properties on a detached mount. */ + attr.userns_fd = get_userns_fd(0, 10000, 10000); + if (attr.userns_fd < 0) { + log_stderr("failure: get_userns_fd"); + goto out; + } + + open_tree_fd = sys_open_tree(t_dir1_fd, "", + AT_EMPTY_PATH | + AT_NO_AUTOMOUNT | + AT_SYMLINK_NOFOLLOW | + OPEN_TREE_CLOEXEC | + OPEN_TREE_CLONE); + if (open_tree_fd < 0) { + log_stderr("failure: sys_open_tree"); + goto out; + } + + if (sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr))) { + log_stderr("failure: sys_mount_setattr"); + goto out; + } + + supported = openat_tmpfile_supported(open_tree_fd); + + /* Only umask with S_IXGRP because inode strip S_ISGID will check mode + * whether has group execute or search permission. + */ + umask(S_IXGRP); + mode = umask(S_IXGRP); + if (!(mode & S_IXGRP)) + die("failure: umask"); + + /* Below we verify that setgid inheritance for a newly created file or + * directory works correctly. As part of this we need to verify that + * newly created files or directories inherit their gid from their + * parent directory. So we change the parent directorie's gid to 1000 + * and create a file with fs{g,u}id 0 and verify that the newly created + * file and directory inherit gid 1000, not 0. + */ + if (fchownat(t_dir1_fd, "", -1, 1000, AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) { + log_stderr("failure: fchownat"); + goto out; + } + + pid = fork(); + if (pid < 0) { + log_stderr("failure: fork"); + goto out; + } + if (pid == 0) { + if (!switch_userns(attr.userns_fd, 0, 0, false)) + die("failure: switch_userns"); + + if (!caps_down_fsetid()) + die("failure: caps_down_fsetid"); + + /* create regular file via open() */ + file1_fd = openat(open_tree_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID); + if (file1_fd < 0) + die("failure: create"); + + /* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid + * bit needs to be stripped. + */ + if (is_setgid(open_tree_fd, FILE1, 0)) + die("failure: is_setgid"); + + /* create directory */ + if (mkdirat(open_tree_fd, DIR1, 0000)) + die("failure: create"); + + if (xfs_irix_sgid_inherit_enabled()) { + /* We're not in_group_p(). */ + if (is_setgid(open_tree_fd, DIR1, 0)) + die("failure: is_setgid"); + } else { + /* Directories always inherit the setgid bit. */ + if (!is_setgid(open_tree_fd, DIR1, 0)) + die("failure: is_setgid"); + } + + /* create a special file via mknodat() vfs_create */ + if (mknodat(open_tree_fd, FILE2, S_IFREG | S_ISGID | S_IXGRP, 0)) + die("failure: mknodat"); + + if (is_setgid(open_tree_fd, FILE2, 0)) + die("failure: is_setgid"); + + /* create a whiteout device via mknodat() vfs_mknod */ + if (mknodat(open_tree_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, 0)) + die("failure: mknodat"); + + if (is_setgid(open_tree_fd, CHRDEV1, 0)) + die("failure: is_setgid"); + + if (unlinkat(open_tree_fd, FILE1, 0)) + die("failure: delete"); + + if (unlinkat(open_tree_fd, DIR1, AT_REMOVEDIR)) + die("failure: delete"); + + if (unlinkat(open_tree_fd, FILE2, 0)) + die("failure: delete"); + + if (unlinkat(open_tree_fd, CHRDEV1, 0)) + die("failure: delete"); + + /* create tmpfile via filesystem tmpfile api */ + if (supported) { + tmpfile_fd = openat(open_tree_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID); + if (tmpfile_fd < 0) + die("failure: create"); + /* link the temporary file into the filesystem, making it permanent */ + snprintf(path, PATH_MAX, "/proc/self/fd/%d", tmpfile_fd); + if (linkat(AT_FDCWD, path, open_tree_fd, FILE3, AT_SYMLINK_FOLLOW)) + die("failure: linkat"); + if (close(tmpfile_fd)) + die("failure: close"); + if (is_setgid(open_tree_fd, FILE3, 0)) + die("failure: is_setgid"); + if (unlinkat(open_tree_fd, FILE3, 0)) + die("failure: delete"); + } + + exit(EXIT_SUCCESS); + } + if (wait_for_pid(pid)) + goto out; + + fret = 0; + log_debug("Ran test"); +out: + safe_close(attr.userns_fd); + safe_close(file1_fd); + safe_close(open_tree_fd); + + return fret; +} + static void usage(void) { fprintf(stderr, "Description:\n"); @@ -14164,6 +14652,7 @@ static void usage(void) fprintf(stderr, "--test-nested-userns Run nested userns idmapped mount testsuite\n"); fprintf(stderr, "--test-btrfs Run btrfs specific idmapped mount testsuite\n"); fprintf(stderr, "--test-setattr-fix-968219708108 Run setattr regression tests\n"); + fprintf(stderr, "--test-setgid-umask Run setgid create umask test\n"); _exit(EXIT_SUCCESS); } @@ -14181,6 +14670,7 @@ static const struct option longopts[] = { {"test-nested-userns", no_argument, 0, 'n'}, {"test-btrfs", no_argument, 0, 'b'}, {"test-setattr-fix-968219708108", no_argument, 0, 'i'}, + {"test-setgid-umask", no_argument, 0, 'u'}, {NULL, 0, 0, 0}, }; @@ -14278,6 +14768,12 @@ struct t_idmapped_mounts t_setattr_fix_968219708108[] = { { setattr_fix_968219708108, true, "test that setattr works correctly", }, }; +struct t_idmapped_mounts t_setgid_umask[] = { + { setgid_create_umask, false, "create operations by using umask in directories with setgid bit set", }, + { setgid_create_umask_idmapped, true, "create operations by using umask in directories with setgid bit set on idmapped mount", }, + { setgid_create_umask_idmapped_in_userns, true, "create operations by using umask in directories with setgid bit set on idmapped mounts inside userns", }, +}; + static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size) { int i; @@ -14355,7 +14851,7 @@ int main(int argc, char *argv[]) int index = 0; bool supported = false, test_btrfs = false, test_core = false, test_fscaps_regression = false, test_nested_userns = false, - test_setattr_fix_968219708108 = false; + test_setattr_fix_968219708108 = false, test_setgid_umask = false; while ((ret = getopt_long_only(argc, argv, "", longopts, &index)) != -1) { switch (ret) { @@ -14392,6 +14888,9 @@ int main(int argc, char *argv[]) case 'i': test_setattr_fix_968219708108 = true; break; + case 'u': + test_setgid_umask = true; + break; case 'h': /* fallthrough */ default: @@ -14463,6 +14962,10 @@ int main(int argc, char *argv[]) ARRAY_SIZE(t_setattr_fix_968219708108))) goto out; + if (test_setgid_umask && + !run_test(t_setgid_umask, ARRAY_SIZE(t_setgid_umask))) + goto out; + fret = EXIT_SUCCESS; out: diff --git a/tests/generic/680 b/tests/generic/680 new file mode 100755 index 00000000..aa9c7375 --- /dev/null +++ b/tests/generic/680 @@ -0,0 +1,26 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved. +# +# FS QA Test 680 +# +# Test that idmapped mounts setgid's behave correctly when using umask. +# +. ./common/preamble +_begin_fstest auto quick cap idmapped mount perms rw unlink + +# Import common functions. +. ./common/filter + +# real QA test starts here + +_supported_fs generic +_require_test + +echo "Silence is golden" + +$here/src/idmapped-mounts/idmapped-mounts --test-setgid-umask --device "$TEST_DEV" \ + --mount "$TEST_DIR" --fstype "$FSTYP" + +status=$? +exit diff --git a/tests/generic/680.out b/tests/generic/680.out new file mode 100644 index 00000000..f4950cda --- /dev/null +++ b/tests/generic/680.out @@ -0,0 +1,2 @@ +QA output created by 680 +Silence is golden
The current_umask() is stripped from the mode directly in the vfs if the filesystem either doesn't support acls or the filesystem has been mounted without posic acl support. If the filesystem does support acls then current_umask() stripping is deferred to posix_acl_create(). So when the filesystem calls posix_acl_create() and there are no acls set or not supported then current_umask() will be stripped. Here we only use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly. Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- src/idmapped-mounts/idmapped-mounts.c | 505 +++++++++++++++++++++++++- tests/generic/680 | 26 ++ tests/generic/680.out | 2 + 3 files changed, 532 insertions(+), 1 deletion(-) create mode 100755 tests/generic/680 create mode 100644 tests/generic/680.out