diff mbox series

[v2,6/6] idmapped-mounts: Add open with O_TMPFILE operation in setgid test

Message ID 1649333375-2599-6-git-send-email-xuyang2018.jy@fujitsu.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/6] idmapped-mount: split setgid test from test-core | expand

Commit Message

Yang Xu (Fujitsu) April 7, 2022, 12:09 p.m. UTC 
Since we can create temp file by using O_TMPFILE flag and filesystem driver also
has this api, we should also check this operation whether strip S_ISGID.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 src/idmapped-mounts/idmapped-mounts.c | 155 +++++++++++++++++++++++++-
 1 file changed, 154 insertions(+), 1 deletion(-)

Comments

Christian Brauner April 7, 2022, 1:43 p.m. UTC | #1 
On Thu, Apr 07, 2022 at 08:09:35PM +0800, Yang Xu wrote:
> Since we can create temp file by using O_TMPFILE flag and filesystem driver also
> has this api, we should also check this operation whether strip S_ISGID.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---

This is a great addition, thanks!
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Yang Xu (Fujitsu) April 8, 2022, 3:58 a.m. UTC | #2 
on 2022/4/7 21:43, Christian Brauner wrote:
> On Thu, Apr 07, 2022 at 08:09:35PM +0800, Yang Xu wrote:
>> Since we can create temp file by using O_TMPFILE flag and filesystem driver also
>> has this api, we should also check this operation whether strip S_ISGID.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>
> This is a great addition, thanks!
> Reviewed-by: Christian Brauner (Microsoft)<brauner@kernel.org>
Thanks for your review.

@Darrick @Eryu @Zorro
just a kindly question, not a push
xfstests has not update for 3 weeks and Eryu will take off maintainer.

Zorro lang has apply for this job. So when does xfstest can work well?

Best Regards
Yang Xu
Zorro Lang April 8, 2022, 7:34 a.m. UTC | #3 
On Fri, Apr 08, 2022 at 03:58:27AM +0000, xuyang2018.jy@fujitsu.com wrote:
> on 2022/4/7 21:43, Christian Brauner wrote:
> > On Thu, Apr 07, 2022 at 08:09:35PM +0800, Yang Xu wrote:
> >> Since we can create temp file by using O_TMPFILE flag and filesystem driver also
> >> has this api, we should also check this operation whether strip S_ISGID.
> >>
> >> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
> >> ---
> >
> > This is a great addition, thanks!
> > Reviewed-by: Christian Brauner (Microsoft)<brauner@kernel.org>
> Thanks for your review.
> 
> @Darrick @Eryu @Zorro
> just a kindly question, not a push
> xfstests has not update for 3 weeks and Eryu will take off maintainer.
> 
> Zorro lang has apply for this job. So when does xfstest can work well?

Hi Yang,

Sorry for the delay, I'm still waiting for the job handover, I have no
permission to push to kernel.org now. The transfer might need a few days/week,
Eryu is busy on other things :) He might merge the current patches on queue
when he get free time, then we can start the handover step by step.

Thanks,
Zorro

> 
> Best Regards
> Yang Xu
Yang Xu (Fujitsu) April 8, 2022, 7:55 a.m. UTC | #4 
on 2022/4/8 15:34, Zorro Lang wrote:
> On Fri, Apr 08, 2022 at 03:58:27AM +0000, xuyang2018.jy@fujitsu.com wrote:
>> on 2022/4/7 21:43, Christian Brauner wrote:
>>> On Thu, Apr 07, 2022 at 08:09:35PM +0800, Yang Xu wrote:
>>>> Since we can create temp file by using O_TMPFILE flag and filesystem driver also
>>>> has this api, we should also check this operation whether strip S_ISGID.
>>>>
>>>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>>>> ---
>>>
>>> This is a great addition, thanks!
>>> Reviewed-by: Christian Brauner (Microsoft)<brauner@kernel.org>
>> Thanks for your review.
>>
>> @Darrick @Eryu @Zorro
>> just a kindly question, not a push
>> xfstests has not update for 3 weeks and Eryu will take off maintainer.
>>
>> Zorro lang has apply for this job. So when does xfstest can work well?
>
> Hi Yang,
>
> Sorry for the delay, I'm still waiting for the job handover, I have no
> permission to push to kernel.org now. The transfer might need a few days/week,
> Eryu is busy on other things :) He might merge the current patches on queue
> when he get free time, then we can start the handover step by step.
Thanks for your reponse.

Best Regards
Yang Xu
>
> Thanks,
> Zorro
>
>>
>> Best Regards
>> Yang Xu
>
diff mbox series

Patch

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index 8f292228..362b8820 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -337,6 +337,24 @@  out:
 	return fret;
 }
 
+static bool openat_tmpfile_supported(int dirfd)
+{
+	int fd = -1;
+
+	fd = openat(dirfd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
+	if (fd == -1) {
+		if (errno == ENOTSUP)
+			return false;
+		else
+			return log_errno(false, "failure: create");
+	}
+
+	if (close(fd))
+		log_stderr("failure: close");
+
+	return true;
+}
+
 /* __expected_uid_gid - check whether file is owned by the provided uid and gid */
 static bool __expected_uid_gid(int dfd, const char *path, int flags,
 			       uid_t expected_uid, gid_t expected_gid, bool log)
@@ -7841,7 +7859,10 @@  static int setgid_create(void)
 {
 	int fret = -1;
 	int file1_fd = -EBADF;
+	int tmpfile_fd = -EBADF;
 	pid_t pid;
+	bool supported = false;
+	char path[PATH_MAX];
 
 	if (!caps_supported())
 		return 0;
@@ -7866,6 +7887,8 @@  static int setgid_create(void)
 		goto out;
 	}
 
+	supported = openat_tmpfile_supported(t_dir1_fd);
+
 	pid = fork();
 	if (pid < 0) {
 		log_stderr("failure: fork");
@@ -7883,9 +7906,24 @@  static int setgid_create(void)
 		if (!is_setgid(t_dir1_fd, FILE1, 0))
 			die("failure: is_setgid");
 
+		/* 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, FILE2, AT_SYMLINK_FOLLOW))
+				die("failure: linkat");
+			if (close(tmpfile_fd))
+				die("failure: close");
+			if (!is_setgid(t_dir1_fd, FILE2, 0))
+				die("failure: is_setgid");
+		}
+
 		/* create directory */
 		if (mkdirat(t_dir1_fd, DIR1, 0000))
-			die("failure: create");
+			die("failure: mkdirat");
 
 		/* Directories always inherit the setgid bit. */
 		if (!is_setgid(t_dir1_fd, DIR1, 0))
@@ -7908,6 +7946,9 @@  static int setgid_create(void)
 		if (!expected_uid_gid(t_dir1_fd, FILE1, 0, 0, 0))
 			die("failure: check ownership");
 
+		if (supported && !expected_uid_gid(t_dir1_fd, FILE2, 0, 0, 0))
+			die("failure: check ownership");
+
 		if (!expected_uid_gid(t_dir1_fd, DIR1 "/" FILE1, 0, 0, 0))
 			die("failure: check ownership");
 
@@ -7920,6 +7961,9 @@  static int setgid_create(void)
 		if (unlinkat(t_dir1_fd, FILE1, 0))
 			die("failure: delete");
 
+		if (supported && unlinkat(t_dir1_fd, FILE2, 0))
+			die("failure: delete");
+
 		if (unlinkat(t_dir1_fd, DIR1 "/" FILE1, 0))
 			die("failure: delete");
 
@@ -7957,6 +8001,21 @@  static int setgid_create(void)
 		if (is_setgid(t_dir1_fd, FILE1, 0))
 			die("failure: is_setgid");
 
+		/* 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, FILE2, AT_SYMLINK_FOLLOW))
+				die("failure: linkat");
+			if (close(tmpfile_fd))
+				die("failure: close");
+			if (is_setgid(t_dir1_fd, FILE2, 0))
+				die("failure: is_setgid");
+		}
+
 		/* create directory */
 		if (mkdirat(t_dir1_fd, DIR1, 0000))
 			die("failure: create");
@@ -7992,6 +8051,9 @@  static int setgid_create(void)
 		if (!expected_uid_gid(t_dir1_fd, FILE1, 0, 0, 0))
 			die("failure: check ownership");
 
+		if (supported && !expected_uid_gid(t_dir1_fd, FILE2, 0, 0, 0))
+			die("failure: check ownership");
+
 		/*
 		 * In setgid directories newly created directories always
 		 * inherit the gid from the parent directory. Verify that the
@@ -8009,6 +8071,9 @@  static int setgid_create(void)
 		if (unlinkat(t_dir1_fd, FILE1, 0))
 			die("failure: delete");
 
+		if (supported && unlinkat(t_dir1_fd, FILE2, 0))
+			die("failure: delete");
+
 		if (unlinkat(t_dir1_fd, DIR1 "/" FILE1, 0))
 			die("failure: delete");
 
@@ -8083,7 +8148,10 @@  static int setgid_create_idmapped(void)
 	struct mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
+	int tmpfile_fd = -EBADF;
 	pid_t pid;
+	bool supported = false;
+	char path[PATH_MAX];
 
 	if (!caps_supported())
 		return 0;
@@ -8131,6 +8199,7 @@  static int setgid_create_idmapped(void)
 		goto out;
 	}
 
+	supported = openat_tmpfile_supported(open_tree_fd);
 	pid = fork();
 	if (pid < 0) {
 		log_stderr("failure: fork");
@@ -8151,6 +8220,21 @@  static int setgid_create_idmapped(void)
 		if (is_setgid(open_tree_fd, FILE1, 0))
 			die("failure: is_setgid");
 
+		/* 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, FILE2, AT_SYMLINK_FOLLOW))
+				die("failure: linkat");
+			if (close(tmpfile_fd))
+				die("failure: close");
+			if (is_setgid(open_tree_fd, FILE2, 0))
+				die("failure: is_setgid");
+		}
+
 		/* create directory */
 		if (mkdirat(open_tree_fd, DIR1, 0000))
 			die("failure: create");
@@ -8173,6 +8257,9 @@  static int setgid_create_idmapped(void)
 		if (!expected_uid_gid(open_tree_fd, FILE1, 0, 10000, 10000))
 			die("failure: check ownership");
 
+		if (supported && !expected_uid_gid(open_tree_fd, FILE2, 0, 10000, 10000))
+			die("failure: check ownership");
+
 		/*
 		 * In setgid directories newly created directories always
 		 * inherit the gid from the parent directory. Verify that the
@@ -8254,7 +8341,10 @@  static int setgid_create_idmapped_in_userns(void)
 	struct mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
+	int tmpfile_fd = -EBADF;
 	pid_t pid;
+	bool supported = false;
+	char path[PATH_MAX];
 
 	if (!caps_supported())
 		return 0;
@@ -8302,6 +8392,7 @@  static int setgid_create_idmapped_in_userns(void)
 		goto out;
 	}
 
+	supported = openat_tmpfile_supported(open_tree_fd);
 	pid = fork();
 	if (pid < 0) {
 		log_stderr("failure: fork");
@@ -8322,6 +8413,21 @@  static int setgid_create_idmapped_in_userns(void)
 		if (!is_setgid(open_tree_fd, FILE1, 0))
 			die("failure: is_setgid");
 
+		/* 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, FILE2, AT_SYMLINK_FOLLOW))
+				die("failure: linkat");
+			if (close(tmpfile_fd))
+				die("failure: close");
+			if (!is_setgid(open_tree_fd, FILE2, 0))
+				die("failure: is_setgid");
+		}
+
 		/* create directory */
 		if (mkdirat(open_tree_fd, DIR1, 0000))
 			die("failure: create");
@@ -8346,6 +8452,9 @@  static int setgid_create_idmapped_in_userns(void)
 		if (!expected_uid_gid(open_tree_fd, FILE1, 0, 0, 0))
 			die("failure: check ownership");
 
+		if (supported && !expected_uid_gid(open_tree_fd, FILE2, 0, 0, 0))
+			die("failure: check ownership");
+
 		if (!expected_uid_gid(open_tree_fd, DIR1, 0, 0, 0))
 			die("failure: check ownership");
 
@@ -8357,6 +8466,8 @@  static int setgid_create_idmapped_in_userns(void)
 
 		if (unlinkat(open_tree_fd, FILE1, 0))
 			die("failure: delete");
+		if (supported && unlinkat(open_tree_fd, FILE2, 0))
+			die("failure: delete");
 		if (unlinkat(open_tree_fd, DIR1 "/" FILE1, 0))
 			die("failure: delete");
 		if (unlinkat(open_tree_fd, CHRDEV1, 0))
@@ -8410,6 +8521,21 @@  static int setgid_create_idmapped_in_userns(void)
 		if (is_setgid(open_tree_fd, FILE1, 0))
 			die("failure: is_setgid");
 
+		/* 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, FILE2, AT_SYMLINK_FOLLOW))
+				die("failure: linkat");
+			if (close(tmpfile_fd))
+				die("failure: close");
+			if (is_setgid(open_tree_fd, FILE2, 0))
+				die("failure: is_setgid");
+		}
+
 		/* create directory */
 		if (mkdirat(open_tree_fd, DIR1, 0000))
 			die("failure: create");
@@ -8446,6 +8572,9 @@  static int setgid_create_idmapped_in_userns(void)
 		if (!expected_uid_gid(open_tree_fd, FILE1, 0, 0, 1000))
 			die("failure: check ownership");
 
+		if (supported && !expected_uid_gid(open_tree_fd, FILE2, 0, 0, 1000))
+			die("failure: check ownership");
+
 		/*
 		 * In setgid directories newly created directories always
 		 * inherit the gid from the parent directory. Verify that the
@@ -8463,6 +8592,9 @@  static int setgid_create_idmapped_in_userns(void)
 		if (unlinkat(open_tree_fd, FILE1, 0))
 			die("failure: delete");
 
+		if (supported && unlinkat(open_tree_fd, FILE2, 0))
+			die("failure: delete");
+
 		if (unlinkat(open_tree_fd, DIR1 "/" FILE1, 0))
 			die("failure: delete");
 
@@ -8515,6 +8647,21 @@  static int setgid_create_idmapped_in_userns(void)
 		if (is_setgid(open_tree_fd, FILE1, 0))
 			die("failure: is_setgid");
 
+		/* 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, FILE2, AT_SYMLINK_FOLLOW))
+				die("failure: linkat");
+			if (close(tmpfile_fd))
+				die("failure: close");
+			if (is_setgid(open_tree_fd, FILE2, 0))
+				die("failure: is_setgid");
+		}
+
 		/* create directory */
 		if (mkdirat(open_tree_fd, DIR1, 0000))
 			die("failure: create");
@@ -8546,6 +8693,9 @@  static int setgid_create_idmapped_in_userns(void)
 		if (!expected_uid_gid(open_tree_fd, FILE1, 0, 0, 0))
 			die("failure: check ownership");
 
+		if (supported && !expected_uid_gid(open_tree_fd, FILE2, 0, 0, 0))
+			die("failure: check ownership");
+
 		if (!expected_uid_gid(open_tree_fd, DIR1, 0, 0, 0))
 			die("failure: check ownership");
 
@@ -8558,6 +8708,9 @@  static int setgid_create_idmapped_in_userns(void)
 		if (unlinkat(open_tree_fd, FILE1, 0))
 			die("failure: delete");
 
+		if (supported && unlinkat(open_tree_fd, FILE2, 0))
+			die("failure: delete");
+
 		if (unlinkat(open_tree_fd, DIR1 "/" FILE1, 0))
 			die("failure: delete");