fstests: add idmapped mounts tests

From: Christian Brauner <[email protected]>

Regression test to verify that creating a series of detached mounts, attaching them to the filesystem, and unmounting them does not trigger an integer overflow in ns->mounts causing the kernel to block any new mounts in count_mounts() and returning ENOSPC because it falsely assumes that the maximum number of mounts in the mount namespace has been reached, i.e. it thinks it can't fit the new mounts into the mount namespace anymore.

The test is written in a way that it will leave the host's mount namespace intact so we are sure to never make the host's mount namespace unuseable!

Link https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557

Cc Eryu Guan <[email protected]>

Cc David Howells <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner <[email protected]>

---
/* v1 - v8 */
patch not present

/* v9 */
- Christian Brauner <[email protected]>:
- Rebased on current master.

/* v10 */
- Eryu Guan <[email protected]>:
- Add missing checks whether test is supported.
- Move status=$? assignment up.
---

Quoted 27 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Regression test to verify that creating a series of detached mounts, attaching them to the filesystem, and unmounting them does not trigger an integer overflow in ns->mounts causing the kernel to block any new mounts in count_mounts() and returning ENOSPC because it falsely assumes that the maximum number of mounts in the mount namespace has been reached, i.e. it thinks it can't fit the new mounts into the mount namespace anymore.

The test is written in a way that it will leave the host's mount namespace intact so we are sure to never make the host's mount namespace unuseable!

Link https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557

Cc Eryu Guan <[email protected]>

Cc David Howells <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner <[email protected]>

---
/* v1 - v8 */
patch not present

/* v9 */
- Christian Brauner <[email protected]>:
- Rebased on current master.

Quoted 5 lines

/* v10 */
- Eryu Guan <[email protected]>:
- Add missing checks whether test is supported.
- Move status=$? assignment up.
---

Technical nit: why did you add this extra --- line?
It causes all the patch changelog to appear in the commit message. I don't think that was your intention? and I don't think it adds valuable into to git log.

Quoted 34 lines · Amir Goldstein

Quoted 32 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Regression test to verify that creating a series of detached mounts, attaching them to the filesystem, and unmounting them does not trigger an integer overflow in ns->mounts causing the kernel to block any new mounts in count_mounts() and returning ENOSPC because it falsely assumes that the maximum number of mounts in the mount namespace has been reached, i.e. it thinks it can't fit the new mounts into the mount namespace anymore.

The test is written in a way that it will leave the host's mount namespace intact so we are sure to never make the host's mount namespace unuseable!

Link https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557

Cc Eryu Guan <[email protected]>

Cc David Howells <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner <[email protected]>

---
/* v1 - v8 */
patch not present

/* v9 */
- Christian Brauner <[email protected]>:
- Rebased on current master.

/* v10 */
- Eryu Guan <[email protected]>:
- Add missing checks whether test is supported.
- Move status=$? assignment up.
---

Quoted 4 lines

Technical nit: why did you add this extra --- line?
It causes all the patch changelog to appear in the commit message. I don't think that was your intention? and I don't think it adds valuable into to git log.

I've not done that in any of the other patches or on any other patches I ever wrote so I think this was just a copy-paste error when I updated the changelog.

Quoted 41 lines · Christian Brauner

Quoted 38 lines · Amir Goldstein

Quoted 27 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Regression test to verify that creating a series of detached mounts, attaching them to the filesystem, and unmounting them does not trigger an integer overflow in ns->mounts causing the kernel to block any new mounts in count_mounts() and returning ENOSPC because it falsely assumes that the maximum number of mounts in the mount namespace has been reached, i.e. it thinks it can't fit the new mounts into the mount namespace anymore.

The test is written in a way that it will leave the host's mount namespace intact so we are sure to never make the host's mount namespace unuseable!

Link https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557

Cc Eryu Guan <[email protected]>

Cc David Howells <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner <[email protected]>

---
/* v1 - v8 */
patch not present

/* v9 */
- Christian Brauner <[email protected]>:
- Rebased on current master.

Quoted 5 lines

/* v10 */
- Eryu Guan <[email protected]>:
- Add missing checks whether test is supported.
- Move status=$? assignment up.
---

Technical nit: why did you add this extra --- line?
It causes all the patch changelog to appear in the commit message. I don't think that was your intention? and I don't think it adds valuable into to git log.

Quoted 4 lines

I've not done that in any of the other patches or on any other patches I ever wrote so I think this was just a copy-paste error when I updated the changelog.

Please note this glitch happened in all the patches in the series...

Quoted 43 lines · Amir Goldstein

Quoted 41 lines · Christian Brauner

Quoted 38 lines · Amir Goldstein

Quoted 27 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Regression test to verify that creating a series of detached mounts, attaching them to the filesystem, and unmounting them does not trigger an integer overflow in ns->mounts causing the kernel to block any new mounts in count_mounts() and returning ENOSPC because it falsely assumes that the maximum number of mounts in the mount namespace has been reached, i.e. it thinks it can't fit the new mounts into the mount namespace anymore.

The test is written in a way that it will leave the host's mount namespace intact so we are sure to never make the host's mount namespace unuseable!

Link https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557

Cc Eryu Guan <[email protected]>

Cc David Howells <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner <[email protected]>

---
/* v1 - v8 */
patch not present

/* v9 */
- Christian Brauner <[email protected]>:
- Rebased on current master.

Quoted 5 lines

/* v10 */
- Eryu Guan <[email protected]>:
- Add missing checks whether test is supported.
- Move status=$? assignment up.
---

Technical nit: why did you add this extra --- line?
It causes all the patch changelog to appear in the commit message. I don't think that was your intention? and I don't think it adds valuable into to git log.

Quoted 6 lines

Quoted 4 lines

I've not done that in any of the other patches or on any other patches I ever wrote so I think this was just a copy-paste error when I updated the changelog.

Please note this glitch happened in all the patches in the series...

Oh wait, actually looking at this again I think don't understand just yet. This is pretty common and when you apply the series git am will cut after the first
---

And thinking about it so far I've never heard anyone complain about this. But likely I just misunderstand you and I really borked something.

Quoted 52 lines · Christian Brauner

Quoted 43 lines · Amir Goldstein

Quoted 41 lines · Christian Brauner

Quoted 38 lines · Amir Goldstein

Quoted 27 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Regression test to verify that creating a series of detached mounts, attaching them to the filesystem, and unmounting them does not trigger an integer overflow in ns->mounts causing the kernel to block any new mounts in count_mounts() and returning ENOSPC because it falsely assumes that the maximum number of mounts in the mount namespace has been reached, i.e. it thinks it can't fit the new mounts into the mount namespace anymore.

The test is written in a way that it will leave the host's mount namespace intact so we are sure to never make the host's mount namespace unuseable!

Link https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557

Cc Eryu Guan <[email protected]>

Cc David Howells <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner <[email protected]>

---
/* v1 - v8 */
patch not present

/* v9 */
- Christian Brauner <[email protected]>:
- Rebased on current master.

Quoted 5 lines

/* v10 */
- Eryu Guan <[email protected]>:
- Add missing checks whether test is supported.
- Move status=$? assignment up.
---

Technical nit: why did you add this extra --- line?
It causes all the patch changelog to appear in the commit message. I don't think that was your intention? and I don't think it adds valuable into to git log.

Quoted 6 lines

Quoted 4 lines

I've not done that in any of the other patches or on any other patches I ever wrote so I think this was just a copy-paste error when I updated the changelog.

Please note this glitch happened in all the patches in the series...

Quoted 8 lines

Oh wait, actually looking at this again I think don't understand just yet. This is pretty common and when you apply the series git am will cut after the first
---

And thinking about it so far I've never heard anyone complain about this. But likely I just misunderstand you and I really borked something.

I see. The reason I brought this up is because I fetched your branch from
https://github.com/brauner/xfstests/ idmapped_mounts_v2
and it has this changelog visible in git log.

So I am not sure how that came to be.

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Quoted 10 lines · Christian Brauner

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

Quoted 4 lines

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi.

Quoted 17 lines · Christian Brauner

Quoted 14 lines · Christian Brauner

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Quoted 3 lines

Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi.

Since 2/6 got dropped, I'll write a small nit here which is also

relevant to the rest of the series:

tests/generic/group +1 -0

@@ -634,3 +634,4 @@
 629 auto quick rw copy_range
 630 auto quick rw dedupe clone
 631 auto quick mount
+632 auto atime attr cap io_uring mount perms quick rw unlink

This is a mouthful of test tags, but that doesn't hurt.
I would personally not bother with obscure tags like 'unlink' but whatever.

Two things I would request are:
1. Keep 'auto quick' before all other tags. There is no strong rule about this format, but that's the common practice and it makes sense IMO because -g auto and -g quick are the far more commonly used groups of tests, so it's convenient to be able to 'eye grep' those tests in
the group file.
2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) This would be used for running tests with -g idmapped for quick sanity when modifiying idmapped mounts related code

Quoted 19 lines · Amir Goldstein

Quoted 17 lines · Christian Brauner

Quoted 14 lines · Christian Brauner

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Quoted 5 lines

Quoted 3 lines

Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi.

Since 2/6 got dropped, I'll write a small nit here which is also

You could still pull it from above. I don't think resending would retain the patch afaict until vger has been ported by Konstantin.

Quoted 13 lines

relevant to the rest of the series:

tests/generic/group +1 -0

@@ -634,3 +634,4 @@
 629 auto quick rw copy_range
 630 auto quick rw dedupe clone
 631 auto quick mount
+632 auto atime attr cap io_uring mount perms quick rw unlink

This is a mouthful of test tags, but that doesn't hurt.
I would personally not bother with obscure tags like 'unlink' but whatever.

Quoted 9 lines

Two things I would request are:
1. Keep 'auto quick' before all other tags. There is no strong rule about this format, but that's the common practice and it makes sense IMO because -g auto and -g quick are the far more commonly used groups of tests, so it's convenient to be able to 'eye grep' those tests in
the group file.
2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) This would be used for running tests with -g idmapped for quick sanity when modifiying idmapped mounts related code

Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in charge of applying it?)

Quoted 43 lines · Christian Brauner

Quoted 19 lines · Amir Goldstein

Quoted 17 lines · Christian Brauner

Quoted 14 lines · Christian Brauner

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Quoted 5 lines

Quoted 3 lines

Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi.

Since 2/6 got dropped, I'll write a small nit here which is also

You could still pull it from above. I don't think resending would retain the patch afaict until vger has been ported by Konstantin.

Quoted 13 lines

relevant to the rest of the series:

tests/generic/group +1 -0

@@ -634,3 +634,4 @@
 629 auto quick rw copy_range
 630 auto quick rw dedupe clone
 631 auto quick mount
+632 auto atime attr cap io_uring mount perms quick rw unlink

This is a mouthful of test tags, but that doesn't hurt.
I would personally not bother with obscure tags like 'unlink' but whatever.

Quoted 12 lines

Quoted 9 lines

Two things I would request are:
1. Keep 'auto quick' before all other tags. There is no strong rule about this format, but that's the common practice and it makes sense IMO because -g auto and -g quick are the far more commonly used groups of tests, so it's convenient to be able to 'eye grep' those tests in
the group file.
2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) This would be used for running tests with -g idmapped for quick sanity when modifiying idmapped mounts related code

Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in charge of applying it?)

He will.

Meanwhile, found a bug in Makefile:

src/idmapped-mounts/Makefile +2 -2

@@ -25,11 +25,11 @@ depend: .dep
 
 include $(BUILDRULES)
 
-idmapped-mounts:
+idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS)
 $(LDFLAGS) $(LDLIBS)
 
-mount-idmapped:
+mount-idmapped: $(CFILES_MOUNT_IDMAPPED)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS)

$(LDFLAGS) $(LDLIBS)
---

And in parsing of /proc/<pid>/ns/user:

src/idmapped-mounts/mount-idmapped.c +0 -0

@@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type,

pid_t pid, const char *buf, s

        int fd = -EBADF, setgroups_fd = -EBADF;
        int fret = -1;
        int ret;

src/idmapped-mounts/mount-idmapped.c +3 -3

-       char path[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) +
+       char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
                  STRLITERALLEN("/setgroups") + 1];
 
        if (geteuid() != 0 && map_type == ID_TYPE_GID) {

@@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap)
 {
        int ret;
        pid_t pid;
-       char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) +
+       char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
                  STRLITERALLEN("/ns/user") + 1];
 
        pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS);

@@ -364,7 +364,7 @@ int main(int argc, char *argv[])
        while ((ret = getopt_long_only(argc, argv, "", longopts,
 &index)) != -1) {
                switch (ret) {
                case 'a':
-                       if (strnequal(optarg, "/proc",
 STRLITERALLEN("/proc/"))) {
+                       if (strnequal(optarg, "/proc/",
 STRLITERALLEN("/proc/"))) {

                                fd_userns = open(optarg, O_RDONLY | O_CLOEXEC);

---

Quoted 115 lines · Amir Goldstein

Quoted 43 lines · Christian Brauner

Quoted 19 lines · Amir Goldstein

Quoted 17 lines · Christian Brauner

Quoted 14 lines · Christian Brauner

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Quoted 5 lines

Quoted 3 lines

Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi.

Since 2/6 got dropped, I'll write a small nit here which is also

You could still pull it from above. I don't think resending would retain the patch afaict until vger has been ported by Konstantin.

Quoted 13 lines

relevant to the rest of the series:

tests/generic/group +1 -0

@@ -634,3 +634,4 @@
 629 auto quick rw copy_range
 630 auto quick rw dedupe clone
 631 auto quick mount
+632 auto atime attr cap io_uring mount perms quick rw unlink

This is a mouthful of test tags, but that doesn't hurt.
I would personally not bother with obscure tags like 'unlink' but whatever.

Quoted 12 lines

Quoted 9 lines

Two things I would request are:
1. Keep 'auto quick' before all other tags. There is no strong rule about this format, but that's the common practice and it makes sense IMO because -g auto and -g quick are the far more commonly used groups of tests, so it's convenient to be able to 'eye grep' those tests in
the group file.
2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) This would be used for running tests with -g idmapped for quick sanity when modifiying idmapped mounts related code

Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in charge of applying it?)

He will.

Meanwhile, found a bug in Makefile:

src/idmapped-mounts/Makefile +2 -2

@@ -25,11 +25,11 @@ depend: .dep
 
 include $(BUILDRULES)
 
-idmapped-mounts:
+idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS)
 $(LDFLAGS) $(LDLIBS)
 
-mount-idmapped:
+mount-idmapped: $(CFILES_MOUNT_IDMAPPED)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS)

$(LDFLAGS) $(LDLIBS)
---

And in parsing of /proc/<pid>/ns/user:

src/idmapped-mounts/mount-idmapped.c +0 -0

@@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type,

pid_t pid, const char *buf, s

        int fd = -EBADF, setgroups_fd = -EBADF;
        int fret = -1;
        int ret;

src/idmapped-mounts/mount-idmapped.c +3 -3

-       char path[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) +
+       char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
                  STRLITERALLEN("/setgroups") + 1];
 
        if (geteuid() != 0 && map_type == ID_TYPE_GID) {

@@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap)
 {
        int ret;
        pid_t pid;
-       char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) +
+       char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
                  STRLITERALLEN("/ns/user") + 1];
 
        pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS);

@@ -364,7 +364,7 @@ int main(int argc, char *argv[])
        while ((ret = getopt_long_only(argc, argv, "", longopts,
 &index)) != -1) {
                switch (ret) {
                case 'a':
-                       if (strnequal(optarg, "/proc",
 STRLITERALLEN("/proc/"))) {
+                       if (strnequal(optarg, "/proc/",
 STRLITERALLEN("/proc/"))) {

                                fd_userns = open(optarg, O_RDONLY | O_CLOEXEC);

Quoted 2 lines

---

And also:

(quoted diff) +5 -3

@@ -402,12 +402,15 @@ int main(int argc, char *argv[])
                exit(EXIT_FAILURE);
        }
 
-       if (!list_empty(&active_map)) {
+       if (!list_empty(&active_map) || fd_userns > 0) {
                struct mount_attr attr = {
                        .attr_set = MOUNT_ATTR_IDMAP,
                };
 
-               attr.userns_fd = get_userns_fd_from_idmap(&active_map);
+               if (fd_userns > 0)
+                       attr.userns_fd = fd_userns;
+               else
+                       attr.userns_fd = get_userns_fd_from_idmap(&active_map);
                if (attr.userns_fd < 0)
                        exit_log("%m - Failed to create user namespace\n");
---

It's a bug in the test helper program, not a bug in the test per-se because the test does not use the
--map-mount=/proc/<pid>/ns/user option.

Quoted 43 lines · Christian Brauner

Quoted 19 lines · Amir Goldstein

Quoted 17 lines · Christian Brauner

Quoted 14 lines · Christian Brauner

Quoted 8 lines · Christian Brauner

From: Christian Brauner <[email protected]>

Hey everyone,

This series is available from:
https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts
https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts
https://github.com/brauner/xfstests/tree/idmapped_mounts

I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled.

Quoted 5 lines

Quoted 3 lines

Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi.

Since 2/6 got dropped, I'll write a small nit here which is also

You could still pull it from above. I don't think resending would retain the patch afaict until vger has been ported by Konstantin.

Quoted 13 lines

relevant to the rest of the series:

tests/generic/group +1 -0

@@ -634,3 +634,4 @@
 629 auto quick rw copy_range
 630 auto quick rw dedupe clone
 631 auto quick mount
+632 auto atime attr cap io_uring mount perms quick rw unlink

This is a mouthful of test tags, but that doesn't hurt.
I would personally not bother with obscure tags like 'unlink' but whatever.

Quoted 12 lines

Quoted 9 lines

Two things I would request are:
1. Keep 'auto quick' before all other tags. There is no strong rule about this format, but that's the common practice and it makes sense IMO because -g auto and -g quick are the far more commonly used groups of tests, so it's convenient to be able to 'eye grep' those tests in
the group file.
2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) This would be used for running tests with -g idmapped for quick sanity when modifiying idmapped mounts related code

Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in charge of applying it?)

He will.

Meanwhile, found a bug in Makefile:

src/idmapped-mounts/Makefile +2 -2

@@ -25,11 +25,11 @@ depend: .dep
 
 include $(BUILDRULES)
 
-idmapped-mounts:
+idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS)
 $(LDFLAGS) $(LDLIBS)
 
-mount-idmapped:
+mount-idmapped: $(CFILES_MOUNT_IDMAPPED)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS)

$(LDFLAGS) $(LDLIBS)

---

And in parsing of /proc/<pid>/ns/user:

src/idmapped-mounts/mount-idmapped.c +0 -0

@@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type,

pid_t pid, const char *buf, s

        int fd = -EBADF, setgroups_fd = -EBADF;
        int fret = -1;
        int ret;

        if (geteuid() != 0 && map_type == ID_TYPE_GID) {

src/idmapped-mounts/mount-idmapped.c +2 -2

@@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap)
 {
        int ret;
        pid_t pid;
-       char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) +
+       char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
                  STRLITERALLEN("/ns/user") + 1];
 
        pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS);

@@ -364,7 +364,7 @@ int main(int argc, char *argv[])
        while ((ret = getopt_long_only(argc, argv, "", longopts,
 &index)) != -1) {
                switch (ret) {
                case 'a':
-                       if (strnequal(optarg, "/proc",
 STRLITERALLEN("/proc/"))) {
+                       if (strnequal(optarg, "/proc/",
 STRLITERALLEN("/proc/"))) {

                                fd_userns = open(optarg, O_RDONLY | O_CLOEXEC);

Quoted 18 lines

Meanwhile, found a bug in Makefile:

src/idmapped-mounts/Makefile +2 -2

@@ -25,11 +25,11 @@ depend: .dep
 
 include $(BUILDRULES)
 
-idmapped-mounts:
+idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS)
 $(LDFLAGS) $(LDLIBS)
 
-mount-idmapped:
+mount-idmapped: $(CFILES_MOUNT_IDMAPPED)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS)

Quoted 1 line

$(LDFLAGS) $(LDLIBS)

Stupid question maybe but what is the reason for this change? It builds fine without those lines.

Building idmapped-mounts
[CC] idmapped-mounts
[CC] mount-idmapped

Quoted 23 lines

Quoted 18 lines

Meanwhile, found a bug in Makefile:

src/idmapped-mounts/Makefile +2 -2

@@ -25,11 +25,11 @@ depend: .dep
 
 include $(BUILDRULES)
 
-idmapped-mounts:
+idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS)
 $(LDFLAGS) $(LDLIBS)
 
-mount-idmapped:
+mount-idmapped: $(CFILES_MOUNT_IDMAPPED)
        @echo "    [CC]    $@"
        $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS)

Quoted 1 line

$(LDFLAGS) $(LDLIBS)

Stupid question maybe but what is the reason for this change? It builds fine without those lines.

Quoted 4 lines

Building idmapped-mounts
[CC] idmapped-mounts
[CC] mount-idmapped

My mistake. You are right. It should be fine as is. I may have messed up something else while building.

You may take the fix or leave it - as you wish.