diff mbox series

[v4,3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

Message ID 1663143142-2283-3-git-send-email-xuyang2018.jy@fujitsu.com
State Superseded
Headers show
Series [v4,1/3] syscalls/creat09: Add umask test condition | expand

Commit Message

Yang Xu Sept. 14, 2022, 8:12 a.m. UTC
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                            |   2 +-
 testcases/kernel/syscalls/openat/.gitignore |   1 +
 testcases/kernel/syscalls/openat/openat04.c | 181 ++++++++++++++++++++
 3 files changed, 183 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/openat/openat04.c

Comments

Cyril Hrubis Sept. 22, 2022, 8:52 a.m. UTC | #1
Hi!
And this one is probably missing some kernel commit tags too, since the
only that that is attached is supposedly in 5.19 but the test still
fails.

I supposed that we should add the 426b4ca2d6a5 to the test tags here as
well, right?
Yang Xu Sept. 22, 2022, 9:06 a.m. UTC | #2
Hi Cyril

>Hi!
> And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.

> I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?

Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.

git tag --contains ac6800e27
v6.0-rc1
v6.0-rc2

Best  Regards
Yang Xu
Cyril Hrubis Sept. 22, 2022, 9:27 a.m. UTC | #3
Hi!
> > And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
> 
> > I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
> 
> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
> 
> git tag --contains ac6800e27
> v6.0-rc1
> v6.0-rc2

Now I'm confused, if I do git describe ac6800e27 it says that it's a
second commit on the top of 5.19-rc7. So shouldn't the the git tag
--contains report 5.19-rc8 and newer? What do I miss?
Yang Xu Sept. 22, 2022, 10:02 a.m. UTC | #4
Hi  Cyril

>> > And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
>>> 
>> > I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
>>>
>> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
>> 
>> git tag --contains ac6800e27
>> v6.0-rc1
>> v6.0-rc2

>Now I'm confused, if I do git describe ac6800e27 it says that it's a second commit on the top of 5.19-rc7. So shouldn't the the git tag --contains report 5.19-rc8 and newer? What do I miss?

I don't know about this. It is a detail in kernel community collaborative work .
But kernel subsystem maintainer should be aware of this. @ Christian Brauner  Can you help Cyril settle this confuse ?

Best Regards
Yang Xu
Christian Brauner Sept. 23, 2022, 10:27 a.m. UTC | #5
On Thu, Sep 22, 2022 at 11:27:43AM +0200, Cyril Hrubis wrote:
> Hi!
> > > And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
> > 
> > > I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
> > 
> > Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
> > 
> > git tag --contains ac6800e27
> > v6.0-rc1
> > v6.0-rc2
> 
> Now I'm confused, if I do git describe ac6800e27 it says that it's a
> second commit on the top of 5.19-rc7. So shouldn't the the git tag
> --contains report 5.19-rc8 and newer? What do I miss?

So, ac6800e279a2 ("fs: Add missing umask strip in vfs_tmpfile") has been
backported to kernels since before the dawn of time. But the the all the
pieces that move setgid handling out of individual filesystems and into
the vfs proper are only >= v6.0-rc1.
Yang Xu Sept. 26, 2022, 1:55 a.m. UTC | #6
Hi Christian

> On Thu, Sep 22, 2022 at 11:27:43AM +0200, Cyril Hrubis wrote:
>> Hi!
>>>> And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
>>>
>>>> I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
>>>
>>> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
>>>
>>> git tag --contains ac6800e27
>>> v6.0-rc1
>>> v6.0-rc2
>>
>> Now I'm confused, if I do git describe ac6800e27 it says that it's a
>> second commit on the top of 5.19-rc7. So shouldn't the the git tag
>> --contains report 5.19-rc8 and newer? What do I miss?
> 
> So, ac6800e279a2 ("fs: Add missing umask strip in vfs_tmpfile") has been
> backported to kernels since before the dawn of time. But the the all the
> pieces that move setgid handling out of individual filesystems and into
> the vfs proper are only >= v6.0-rc1.

I search the commit mesage in the following url,but not find c6800e279a2 
commit

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=v5.19

@Cyril
I guess git describe ac6800e2 will serach the commit id because it is 
merge request"Merge tag 'fs.setgid.v6.0' of 
git://git.kernel.org/pub/scm/linux/kerne/git/brauner/linux"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=426b4ca2d6a5

then it will serach brauner linux fs.setgid. branch
see the following url
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fs.setgid

so git describe ac6800e27 it says that it's a second commit on the top 
of 5.19-rc7.


Best Regards
Yang Xu
Yang Xu Oct. 10, 2022, 6:32 a.m. UTC | #7
Hi Cyril

> Hi Christian
> 
>> On Thu, Sep 22, 2022 at 11:27:43AM +0200, Cyril Hrubis wrote:
>>> Hi!
>>>>> And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
>>>>
>>>>> I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
>>>>
>>>> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
>>>>
>>>> git tag --contains ac6800e27
>>>> v6.0-rc1
>>>> v6.0-rc2
>>>
>>> Now I'm confused, if I do git describe ac6800e27 it says that it's a
>>> second commit on the top of 5.19-rc7. So shouldn't the the git tag
>>> --contains report 5.19-rc8 and newer? What do I miss?
>>
>> So, ac6800e279a2 ("fs: Add missing umask strip in vfs_tmpfile") has been
>> backported to kernels since before the dawn of time. But the the all the
>> pieces that move setgid handling out of individual filesystems and into
>> the vfs proper are only >= v6.0-rc1.
> 
> I search the commit mesage in the following url,but not find c6800e279a2
> commit
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=v5.19
> 
> @Cyril
> I guess git describe ac6800e2 will serach the commit id because it is
> merge request"Merge tag 'fs.setgid.v6.0' of
> git://git.kernel.org/pub/scm/linux/kerne/git/brauner/linux"
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=426b4ca2d6a5
> 
> then it will serach brauner linux fs.setgid. branch
> see the following url
> https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fs.setgid
> 
> so git describe ac6800e27 it says that it's a second commit on the top
> of 5.19-rc7.

So do you still have doubt on this?  I don't think this should block 
this two patches.

Best Regards
Yang Xu
> 
> 
> Best Regards
> Yang Xu
>
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index 9d58e0aa1..cd38a4ddf 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -919,10 +919,10 @@  open12 open12
 open13 open13
 open14 open14
 
-#openat test cases
 openat01 openat01
 openat02 openat02
 openat03 openat03
+openat04 openat04
 
 openat201 openat201
 openat202 openat202
diff --git a/testcases/kernel/syscalls/openat/.gitignore b/testcases/kernel/syscalls/openat/.gitignore
index 2928dae22..2d15872ab 100644
--- a/testcases/kernel/syscalls/openat/.gitignore
+++ b/testcases/kernel/syscalls/openat/.gitignore
@@ -2,3 +2,4 @@ 
 /openat02
 /openat02_child
 /openat03
+/openat04
diff --git a/testcases/kernel/syscalls/openat/openat04.c b/testcases/kernel/syscalls/openat/openat04.c
new file mode 100644
index 000000000..1fbe51c60
--- /dev/null
+++ b/testcases/kernel/syscalls/openat/openat04.c
@@ -0,0 +1,181 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Check setgid strip logic whether works correctly when creating tmpfile under
+ * filesystem without POSIX ACL supported(by using noacl mount option). Test it
+ * with umask S_IXGRP and also check file mode whether has filtered S_IXGRP.
+ *
+ * Fixed in:
+ *
+ *  commit ac6800e279a22b28f4fc21439843025a0d5bf03e
+ *  Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ *  Date:   Thu July 14 14:11:26 2022 +0800
+ *
+ *  fs: Add missing umask strip in vfs_tmpfile
+ *
+ * The most code is pasted form creat09.c.
+ */
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <sys/types.h>
+#include <pwd.h>
+#include <sys/mount.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdio.h>
+#include "tst_test.h"
+#include "tst_uid.h"
+#include "tst_safe_file_at.h"
+
+#define MODE_RWX        0777
+#define MODE_SGID       (S_ISGID|0777)
+#define MNTPOINT	"mntpoint"
+#define WORKDIR		MNTPOINT "/testdir"
+#define OPEN_FILE	"open.tmp"
+
+static gid_t free_gid;
+static int tmpfile_fd = -1, dir_fd = -1, mount_flag;
+static struct passwd *ltpuser;
+
+static void do_mount(const char *source, const char *target,
+	const char *filesystemtype, unsigned long mountflags,
+	const void *data)
+{
+	TEST(mount(source, target, filesystemtype, mountflags, data));
+
+	if (TST_RET == -1 && TST_ERR == EINVAL)
+		tst_brk(TCONF, "Kernel does not support noacl feature");
+
+	if (TST_RET == -1) {
+		tst_brk(TBROK | TTERRNO, "mount(%s, %s, %s, %lu, %p) failed",
+			source, target, filesystemtype, mountflags, data);
+	}
+
+	if (TST_RET)
+		tst_brk(TBROK, "Invalid mount return value %ld", TST_RET);
+
+	mount_flag = 1;
+}
+
+static void open_tmpfile_supported(int dirfd)
+{
+	TEST(openat(dirfd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID));
+
+	if (TST_RET == -1) {
+		if (errno == ENOTSUP)
+			tst_brk(TCONF, "fs doesn't support O_TMPFILE");
+		else
+			tst_brk(TBROK | TTERRNO, "openat(%d, O_TMPFILE) failed", dirfd);
+	}
+
+	if (TST_RET < 0)
+		tst_brk(TBROK, "Invalid openat return value %ld", TST_RET);
+
+	SAFE_CLOSE(TST_RET);
+}
+
+static void setup(void)
+{
+	struct stat buf;
+
+	ltpuser = SAFE_GETPWNAM("nobody");
+
+	do_mount(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "noacl");
+
+	tst_res(TINFO, "User nobody: uid = %d, gid = %d", (int)ltpuser->pw_uid,
+		(int)ltpuser->pw_gid);
+	free_gid = tst_get_free_gid(ltpuser->pw_gid);
+
+	/* Create directories and set permissions */
+	SAFE_MKDIR(WORKDIR, MODE_RWX);
+	dir_fd = SAFE_OPEN(WORKDIR, O_RDONLY, O_DIRECTORY);
+	open_tmpfile_supported(dir_fd);
+
+	SAFE_CHOWN(WORKDIR, ltpuser->pw_uid, free_gid);
+	SAFE_CHMOD(WORKDIR, MODE_SGID);
+	SAFE_STAT(WORKDIR, &buf);
+
+	if (!(buf.st_mode & S_ISGID))
+		tst_brk(TBROK, "%s: Setgid bit not set", WORKDIR);
+
+	if (buf.st_gid != free_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", WORKDIR,
+			buf.st_gid, free_gid);
+	}
+
+	/* Switch user */
+	SAFE_SETGID(ltpuser->pw_gid);
+	SAFE_SETREUID(-1, ltpuser->pw_uid);
+}
+
+static void file_test(int dfd, const char *path, int flags)
+{
+	struct stat buf;
+
+	SAFE_FSTATAT(dfd, path, &buf, flags);
+
+	TST_EXP_EQ_LI(buf.st_gid, free_gid);
+
+	if (buf.st_mode & S_ISGID)
+		tst_res(TFAIL, "%s: Setgid bit is set", path);
+	else
+		tst_res(TPASS, "%s: Setgid bit not set", path);
+
+	if (buf.st_mode & S_IXGRP)
+		tst_res(TFAIL, "%s: S_IXGRP bit is set", path);
+	else
+		tst_res(TPASS, "%s: S_IXGRP bit is not set", path);
+}
+
+static void run(void)
+{
+	char path[PATH_MAX];
+
+	umask(S_IXGRP);
+	tmpfile_fd = SAFE_OPENAT(dir_fd, ".", O_TMPFILE | O_RDWR, MODE_SGID);
+	snprintf(path, PATH_MAX, "/proc/self/fd/%d", tmpfile_fd);
+	SAFE_LINKAT(AT_FDCWD, path, dir_fd, OPEN_FILE, AT_SYMLINK_FOLLOW);
+	file_test(dir_fd, OPEN_FILE, 0);
+	SAFE_CLOSE(tmpfile_fd);
+	/* Cleanup between loops */
+	tst_purge_dir(WORKDIR);
+}
+
+static void cleanup(void)
+{
+	SAFE_SETREUID(-1, 0);
+
+	if (tmpfile_fd >= 0)
+		SAFE_CLOSE(tmpfile_fd);
+	if (dir_fd >= 0)
+		SAFE_CLOSE(dir_fd);
+	if (mount_flag && tst_umount(MNTPOINT))
+		tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.all_filesystems = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.skip_filesystems = (const char*[]) {
+		"exfat",
+		"ntfs",
+		"vfat",
+		NULL
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "ac6800e279a2"},
+		{}
+	},
+};