According to https://github.com/MusicPlayerDaemon/MPD/issues/558 MPD uses O_TMPFILE to create its database, and the file mode of the newly created database is 666. This means umask is ignored. `man 2 open` doesn't state explicit support for umask from O_TMPFILE. Is O_TMPFILE supposed to ignore umask? Or, is it a bug for O_TMPFILE to ignore umask?
1. Can you provide selfcontained c code that demostrates the problem? 2. Is it a regression or always been like that?
1. I don't know how to write C code. I certainly don't know how to write a minimal C example demonstrating the issue. I tried and gave up. 2. I think it has been like that for at least years. MPD had the issue for years. I noticed the issue again recently and submitted it here.
'man 2 open' says that umask should already have an effect for O_CREAT and O_TMPFILE. O_TMPFILE creates nameless files (for example via mkstemp). How do you observe it's effect (fstat?) and why does it matter? Feel free to use the template below to create a reproducer that illustrates a problem. $ cat a.c // $ gcc a.c -o a && ./a #define _GNU_SOURCE /* for O_TMPFILE */ #include <fcntl.h> #include <unistd.h> #include <sys/types.h> #include <sys/stat.h> #include <errno.h> #include <stdio.h> #include <string.h> static const char probe_path[] = "/tmp/probe-open-for-b686142"; static void probe_umask(void) { mode_t current_umask = umask(0); umask(current_umask); printf("current umask = %#o\n", (unsigned)current_umask); } static void probe_open(const char * test_desc, int flags, mode_t mode) { struct stat s; int fd; memset(&s, 0, sizeof(s)); // create target file fd = open(probe_path, flags, mode); close (fd); // check the resulting mode int r = stat(probe_path, &s); int e = errno; // cleanup unlink(probe_path); if (r == 0) printf("%s: in_mode=%#o, resulting_mode=%#o\n", test_desc, (unsigned)mode, (unsigned)s.st_mode); else printf("%s: stat() failed as: %s\n", test_desc, strerror(e)); } static void probe_open_modes(void) { probe_umask(); probe_open("O_CREAT, 777", O_CREAT, 0777); probe_open("O_TMPFILE, 777", O_TMPFILE, 777); probe_open("O_CREAT|O_TMPFILE, 777", O_CREAT|O_TMPFILE, 777); } int main() { // test defaults probe_open_modes(); // test restricted masks umask(0077); probe_open_modes(); } $ gcc a.c -o a && ./a current umask = 022 O_CREAT, 777: in_mode=0777, resulting_mode=0100755 O_TMPFILE, 777: stat() failed as: No such file or directory O_CREAT|O_TMPFILE, 777: stat() failed as: No such file or directory current umask = 077 O_CREAT, 777: in_mode=0777, resulting_mode=0100700 O_TMPFILE, 777: stat() failed as: No such file or directory O_CREAT|O_TMPFILE, 777: stat() failed as: No such file or directory
Oh, I see mpd uses linkat from '/proc/self/fd', that makes sense. Let me craft a shorter and less broken example.
Here is the updated sample. Works as expected (respects umask) on tmpfs, btrfs, xfs and ext4. $ cat a.c // $ gcc a.c -o a && ./a #define _GNU_SOURCE /* for O_TMPFILE */ #include <fcntl.h> #include <unistd.h> #include <sys/types.h> #include <sys/stat.h> #include <errno.h> #include <limits.h> #include <stdio.h> #include <string.h> static char probe_name[] = "file-for-b686142"; static void probe_umask(void) { mode_t current_umask = umask(0); umask(current_umask); printf("current umask = %#o\n", (unsigned)current_umask); } static void probe_open(void) { struct stat fs; struct stat s; int fd; int e; char path[PATH_MAX]; mode_t mode = 0777; memset(&fs, 0, sizeof(fs)); memset(&s, 0, sizeof(s)); // create target file fd = open(".", O_TMPFILE | O_RDWR, mode); fstat(fd, &fs); snprintf(path, PATH_MAX, "/proc/self/fd/%d", fd); linkat(AT_FDCWD, path, AT_FDCWD, probe_name, AT_SYMLINK_FOLLOW); close (fd); stat(probe_name, &s); unlink(probe_name); printf("fstat(): in_mode=%#o, resulting_mode=%#o\n", (unsigned)mode, (unsigned)fs.st_mode); printf("stat(): in_mode=%#o, resulting_mode=%#o\n", (unsigned)mode, (unsigned)s.st_mode); } static void probe_open_modes(void) { probe_umask(); probe_open(); } int main() { // test defaults probe_open_modes(); // test restricted masks umask(0077); probe_open_modes(); } $ gcc a.c -o a && ./a current umask = 022 fstat(): in_mode=0777, resulting_mode=0100755 stat(): in_mode=0777, resulting_mode=0100755 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100700 stat(): in_mode=0777, resulting_mode=0100700
By the way, I use mpd on zfs. > zfs get xattr user-data/data NAME PROPERTY VALUE SOURCE user-data/data xattr sa local > zfs get acltype user-data/data NAME PROPERTY VALUE SOURCE user-data/data acltype posixacl local
What does test from #comment5 yield for you when ran on those filesystems?
On ZFS, I get the following result. > ./a current umask = 022 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777
Looks like you found a zfs-specific bug. CC-ing maintainers. Tl;DR: test from #comment5 demonstrates deviation from shipped linux filesystems and causes O_TMPFILE files relinked into named files have permission mask too broad. I would say it's a security bug.
Please provide 'emerge --info sys-fs/zfs sys-fs/zfs-kmod'.
Confirmed [1] on a virtual machine: FAILS: /zp (ZFS):fails on ZFS WORKS: /home/root (EXT4) On https://github.com/MusicPlayerDaemon/MPD/issues/558 Max Kellermann found out it is a mix of ACL support presence in kernel and filesystem-specific handling of vnode cretion. CC-ing kernel@ as well. I'm leaving this bug to ZFS and kernel Gentoo maintainers. Please decide who is best to own this bug. [1] test of #comment5 program: localhost /zp # gcc a.c -o a && ./a current umask = 022 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777 localhost /zp # cd localhost ~ # /zp/a current umask = 022 fstat(): in_mode=0777, resulting_mode=0100755 stat(): in_mode=0777, resulting_mode=0100755 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100700 stat(): in_mode=0777, resulting_mode=0100700 localhost ~ # mount /dev/sda1 on / type ext4 (rw,relatime) zp on /zp type zfs (rw,xattr,noacl) ...
Created attachment 581946 [details] emerge --info
Created attachment 581948 [details] config-4.19.52-gentoo kernel config
Created attachment 582012 [details] emerge --info sys-fs/zfs sys-fs/zfs-kmod
I'll keep looking with other settings, but I can't reproduce on zfs right now on my pool. ./a.out current umask = 022 fstat(): in_mode=0777, resulting_mode=0100755 stat(): in_mode=0777, resulting_mode=0100755 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100700 stat(): in_mode=0777, resulting_mode=0100700 zroot/ROOT/root on / type zfs (rw,relatime,xattr,posixacl) NAME PROPERTY VALUE SOURCE zroot acltype posixacl local zroot xattr sa local
(In reply to Georgy Yakovlev from comment #15) > I'll keep looking with other settings, but I can't reproduce on zfs right > now on my pool. > > ./a.out > current umask = 022 > fstat(): in_mode=0777, resulting_mode=0100755 > stat(): in_mode=0777, resulting_mode=0100755 > current umask = 077 > fstat(): in_mode=0777, resulting_mode=0100700 > stat(): in_mode=0777, resulting_mode=0100700 > > > zroot/ROOT/root on / type zfs (rw,relatime,xattr,posixacl) > NAME PROPERTY VALUE SOURCE > zroot acltype posixacl local > zroot xattr sa local I guess it's because difference in noacl/posixacl mount options. No idea how to influence it on zfs. Maybe set some filesystem property?
> I guess it's because difference in noacl/posixacl mount options. No idea how > to influence it on zfs. Maybe set some filesystem property? Probably via 'zfs set acltype=noacl zroot/ROOT/root' (or better newly created fliesystem).
Seems to trigger at least for me: localhost /zp # zfs set acltype=posixacl zp localhost /zp # gcc a.c -o a && ./a current umask = 022 fstat(): in_mode=0777, resulting_mode=0100755 stat(): in_mode=0777, resulting_mode=0100755 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100700 stat(): in_mode=0777, resulting_mode=0100700 localhost /zp # zfs set acltype=noacl zp localhost /zp # gcc a.c -o a && ./a current umask = 022 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777
indeed, current umask = 022 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777 current umask = 077 fstat(): in_mode=0777, resulting_mode=0100777 stat(): in_mode=0777, resulting_mode=0100777 on freshly created pool with default settings zfs get acltype tmpfile NAME PROPERTY VALUE SOURCE tmpfile acltype off default tmpfile on /tmpfile type zfs (rw,xattr,noacl)
Looks like there is a pull request against zfs code available: https://github.com/zfsonlinux/zfs/pull/8998 Reassigning to zfs maintainers.
while the initial patch works for me, there's an ongoing discussion. will backport this to 0.8.1 (and maybe 0.7.x) as soon as it gets merged to master upstream after all review and buildbot testing.
just checking in, work upstream still in progress
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=2b66902c411859f19d0ee76b38efcd162d6b3f8f commit 2b66902c411859f19d0ee76b38efcd162d6b3f8f Author: Georgy Yakovlev <gyakovlev@gentoo.org> AuthorDate: 2019-12-13 23:41:16 +0000 Commit: Georgy Yakovlev <gyakovlev@gentoo.org> CommitDate: 2019-12-13 23:42:23 +0000 sys-fs/zfs-kmod: revbump 0.8.2, fix O_TMPFILE umask ignore Bug: https://bugs.gentoo.org/686142 Package-Manager: Portage-2.3.79, Repoman-2.3.17 Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org> sys-fs/zfs-kmod/files/0.8.2-umask_O_TMPFILE.patch | 34 ++++++++++++++++++++++ ...od-0.8.2-r1.ebuild => zfs-kmod-0.8.2-r2.ebuild} | 2 ++ 2 files changed, 36 insertions(+)
The upstream issue was fixed.
Yeah, already in gentoo for 0.8.2 Do we need a backport to 0.7.x ? I haven’t looked yet if the bug is there and moved all my systems to modern branch.
I'm a bit mixed on backporting (and not a zfs user myself). On one hand (no backport) it's obscure enough and not a regression. On the other hand (yes backport) it's a security hole.
I'm also not a fan of going full redhat =D but this both look simple enough change and quite important. I just need to set up non 0.8x machine to test...
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=8f41bec723fda7d56c431ed14fda7cf2c9990da9 commit 8f41bec723fda7d56c431ed14fda7cf2c9990da9 Author: Georgy Yakovlev <gyakovlev@gentoo.org> AuthorDate: 2020-05-15 03:44:16 +0000 Commit: Georgy Yakovlev <gyakovlev@gentoo.org> CommitDate: 2020-05-15 03:49:41 +0000 profiles/package.mask: mask zfs-0.7.x and co Bug: https://bugs.gentoo.org/686142 Bug: https://bugs.gentoo.org/723120 Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org> profiles/package.mask | 8 ++++++++ 1 file changed, 8 insertions(+)
I tried back-porting but never got to run a testsuite on old version with the patch. anyway, time has come, masking 0.7.x releases
old versions removed in https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=b34316d01d3730756778f0b15e7fe6b2c50d255f closing.
