| Message ID | 20240222031018.12281-2-wegao@suse.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Set system default umaks to 0 | expand |
Hi Wei, Petr,
Seems the problem Wei met is the default umask of 0077,
any new files or directories that are created will have their
permission bits modified by this umask.
After looking though what you both discussed, I think maybe
another better choice is to set the umask to '0000' temporarily
before creating the directory, and then restoring the previous
umask right after.
All these operations are just put into cgroup_dir_mk function.
Something like this:
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -368,8 +368,11 @@ static void cgroup_dir_mk(const struct cgroup_dir
*const parent,
new->ctrl_field = parent->ctrl_field;
new->we_created_it = 0;
+ mode_t old_umask = umask(0000);
+
if (!mkdirat(parent->dir_fd, dir_name, 0777)) {
new->we_created_it = 1;
+ umask(old_umask);
goto opendir;
}
Hi Li, > Hi Wei, Petr, > Seems the problem Wei met is the default umask of 0077, > any new files or directories that are created will have their > permission bits modified by this umask. > After looking though what you both discussed, I think maybe > another better choice is to set the umask to '0000' temporarily > before creating the directory, and then restoring the previous > umask right after. > All these operations are just put into cgroup_dir_mk function. LGTM this approach, please send a patch. > Something like this: > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -368,8 +368,11 @@ static void cgroup_dir_mk(const struct cgroup_dir > *const parent, > new->ctrl_field = parent->ctrl_field; > new->we_created_it = 0; > + mode_t old_umask = umask(0000); nit: umask(0) is the same right? Kind regards, Petr > + > if (!mkdirat(parent->dir_fd, dir_name, 0777)) { > new->we_created_it = 1; > + umask(old_umask); > goto opendir; > }
On Fri, Mar 1, 2024 at 4:43 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, > > > Hi Wei, Petr, > > > Seems the problem Wei met is the default umask of 0077, > > any new files or directories that are created will have their > > permission bits modified by this umask. > > > After looking though what you both discussed, I think maybe > > another better choice is to set the umask to '0000' temporarily > > before creating the directory, and then restoring the previous > > umask right after. > > > All these operations are just put into cgroup_dir_mk function. > > LGTM this approach, please send a patch. > Ok, sure. > > + mode_t old_umask = umask(0000); > nit: umask(0) is the same right? > exactly.
diff --git a/testcases/kernel/controllers/cgroup/cgroup_core01.c b/testcases/kernel/controllers/cgroup/cgroup_core01.c index 2e695deed..ad08c74a9 100644 --- a/testcases/kernel/controllers/cgroup/cgroup_core01.c +++ b/testcases/kernel/controllers/cgroup/cgroup_core01.c @@ -76,6 +76,8 @@ static void setup(void) { struct passwd *pw; + umask(0); + pw = SAFE_GETPWNAM("nobody"); nobody_uid = pw->pw_uid; save_uid = geteuid();
When system's default umask is 0077, following error will popup: cgroup_core01.c:50: TBROK: openat(21</sys/fs/cgroup/memory/ltp/test-3519/child_b>, 'tasks', 2, 0): EACCES (13) The reason is default user will create directory without permission for group and other if umask(0077), then error happen if case switch to nobody user. Signed-off-by: Wei Gao <wegao@suse.com> --- testcases/kernel/controllers/cgroup/cgroup_core01.c | 2 ++ 1 file changed, 2 insertions(+)