From: Christian Brauner <christian.brauner@ubuntu.com>
To: Eryu Guan <guan@eryu.me>
Cc: Christian Brauner <brauner@kernel.org>,
fstests@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
"Darrick J . Wong" <djwong@kernel.org>,
David Howells <dhowells@redhat.com>,
Amir Goldstein <amir73il@gmail.com>
Subject: Re: [PATCH v11 2/6] generic/632: add fstests for idmapped mounts
Date: Sun, 28 Mar 2021 21:25:32 +0200 [thread overview]
Message-ID: <20210328192532.jrw23brihkcrc6ok@wittgenstein> (raw)
In-Reply-To: <YGDL9EKPnPIcj6yh@desktop>
On Mon, Mar 29, 2021 at 02:33:24AM +0800, Eryu Guan wrote:
> On Sat, Mar 27, 2021 at 12:18:52PM +0100, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> >
> > Add a test suite to verify the behavior of idmapped mounts. The test
> > suite also includes a range of vfs tests to verify that no regressions
> > are introduced by idmapped mounts. The following tests are currently
> > available with more to come in the future:
> >
> > 01. posix acls on regular and idmapped mounts
> > 02. create operations in user namespace
> > 03. device node creation in user namespace
> > 04. expected ownership on idmapped mounts
> > 05. fscaps on regular mounts
> > 06. fscaps on idmapped mounts
> > 07. fscaps on idmapped mounts in user namespace
> > 08. fscaps on idmapped mounts in user namespace
> > with different id mappings
> > 09. mapped fsids
> > 10. unmapped fsids
> > 11. cross mount hardlink
> > 12. cross idmapped mount hardlink
> > 13. hardlinks from idmapped mounts
> > 14. hardlinks from idmapped mounts in user namespace
> > 15. io_uring
> > 16. io_uring in user namespace
> > 17. io_uring from idmapped mounts
> > 18. io_uring from idmapped mounts in user namespace
> > 19. io_uring from idmapped mounts with unmapped ids
> > 20. io_uring from idmapped mounts with unmapped ids in user namespace
> > 21. following protected symlinks on regular mounts
> > 22. following protected symlinks on idmapped mounts
> > 23. following protected symlinks on idmapped mounts in user namespace
> > 24. cross mount rename
> > 25. cross idmapped mount rename
> > 26. rename from idmapped mounts
> > 27. rename from idmapped mounts in user namespace
> > 28. symlink from regular mounts
> > 29. symlink from idmapped mounts
> > 30. symlink from idmapped mounts in user namespace
> > 31. setid binaries on regular mounts
> > 32. setid binaries on idmapped mounts
> > 33. setid binaries on idmapped mounts in user namespace
> > 34. setid binaries on idmapped mounts in user namespace
> > with different id mappings
> > 35. sticky bit unlink operations on regular mounts
> > 36. sticky bit unlink operations on idmapped mounts
> > 37. sticky bit unlink operations on idmapped mounts in user namespace
> > 38. sticky bit rename operations on regular mounts
> > 39. sticky bit rename operations on idmapped mounts
> > 40. sticky bit rename operations on idmapped mounts in user namespace
> > 41. create operations in directories with setgid bit set
> > 42. create operations in directories with setgid bit set
> > on idmapped mounts
> > 43. create operations in directories with setgid bit set
> > on idmapped mounts in user namespace
> > 44. verify create operations and ownership with racing threads idmapping
> > a mount
> > 45. setattr truncate operations on regular mounts
> > 46. setattr truncate operations on idmapped mounts
> > 47. setattr truncate operations on idmapped mounts in user namespace
> >
> > Here's some sample output when running with DEBUG_TRACE defined:
> >
> > Cc: Amir Goldstein <amir73il@gmail.com>
> > Cc: Eryu Guan <guan@eryu.me>
> > Cc: fstests@vger.kernel.org
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> > /* v2 */ (send out alongside idmapped mounts patchset)
> > test-suite part of the kernel selftest framework
> >
> > /* v3 */ (send out alongside idmapped mounts patchset)
> > patch introduced (ported form kernel selftest framework)
> >
> > /* v4 */ (send out alongside idmapped mounts patchset)
> > - Amir Goldstein <amir73il@gmail.com>:
> > - Don't Cc fstests mailing list on the testing patches for now.
> >
> > - Add setgid creation tests.
> >
> > /* v5 */ (send out alongside idmapped mounts patchset)
> > new base: 2080ad637bbb4d2c24c4d63939799dab178eb407
> >
> > - Christoph Hellwig <hch@lst.de>:
> > - Enable xfs testing.
> > - Improve idmapped mount testing support.
> >
> > - Christian Brauner <christian.brauner@ubuntu.com>:
> > - Fix protected symlink tests.
> > - Introduce xfs specific helpers.
> > - Expand setgid bit tests to xfs.
> >
> > /* v6 */ (send out alongside idmapped mounts patchset)
> > - Christian Brauner <christian.brauner@ubuntu.com>:
> > - After Christoph fixed setgid handling on xfs remove all xfs specific
> > parts apart from irix_sgid_inheritance checking.
> > - Add idmapped-mounts binary to .gitignore
> > - Fix feature testing.
> > - Add truncation tests.
> >
> > /* v7 */ (sent out as a separate patchset)
> > new base: 011bfb01f7f9f8f37c01d0a1ae0b0ca28e96a4f5
> > - Christian Brauner <christian.brauner@ubuntu.com>:
> > - Expand debugging options.
> >
> > /* v8 */
> > - Christoph Hellwig <hch@lst.de>:
> > - Use __supported_fs generic instead of explicitly listing ext4 and xfs.
> > - Remove obsolete comment.
> >
> > /* v9 */
> > - Christian Brauner <christian.brauner@ubuntu.com>:
> > - Rebase on current master.
> >
> > /* v10 */
> > unchanged
> >
> > /* v11 */
> > - Amir Goldstein <amir73il@gmail.com>:
> > - Place "auto" and "quick" tags first for easier "eye grepping".
> > - Add a dedicated "idmapped" tag for idmapped mounts tests.
> > ---
> > .gitignore | 1 +
> > common/rc | 25 +
> > configure.ac | 2 +
> > include/builddefs.in | 1 +
> > m4/Makefile | 1 +
> > m4/package_libcap.m4 | 4 +
> > src/Makefile | 5 +
> > src/detached_mounts_propagation.c | 65 +-
> > src/feature.c | 40 +-
> > src/idmapped-mounts/Makefile | 35 +
> > src/idmapped-mounts/idmapped-mounts.c | 8761 +++++++++++++++++++++++++
> > src/idmapped-mounts/missing.h | 151 +
> > src/idmapped-mounts/utils.c | 134 +
> > src/idmapped-mounts/utils.h | 29 +
> > tests/generic/632 | 42 +
> > tests/generic/632.out | 2 +
> > tests/generic/group | 1 +
> > 17 files changed, 9231 insertions(+), 68 deletions(-)
> > create mode 100644 m4/package_libcap.m4
> > create mode 100644 src/idmapped-mounts/Makefile
> > create mode 100644 src/idmapped-mounts/idmapped-mounts.c
> > create mode 100644 src/idmapped-mounts/missing.h
> > create mode 100644 src/idmapped-mounts/utils.c
> > create mode 100644 src/idmapped-mounts/utils.h
> > create mode 100644 tests/generic/632
> > create mode 100644 tests/generic/632.out
> >
>
> [snip]
>
> > +
> > +/* caps_down - lower all effective caps */
> > +static int caps_down(void)
> > +{
> > + bool fret = false;
>
> When HAVE_SYS_CAPABILITY_H is not defined, i.e. libcap-devel package is
> not installed, so this causes test fails as
>
> @@ -1,2 +1,7 @@
> QA output created by 632
> Silence is golden
> +idmapped-mounts.c: 419: switch_userns - Success - failure: caps_down
> +idmapped-mounts.c: 6617: acls - Success - failure: switch_userns
> +idmapped-mounts.c: 6627: acls - Success - failure: wait_for_pid
> +failure: posix acls on regular mounts
> +idmapped-mounts.c: 805: test_cleanup - Directory not empty - failure: rm_r
>
> Also, please update README to add libcap-devel to "install prerequisite
> packages" list.
>
> And it seems some tests depend on caps_down(), like io_uring tests, it
> depends on caps_down() to drop caps and expect EPERM. It'd be better to
> make libcap-devel optional and don't fail other tests if it's not
> installed.
Ok, fixing this now.
>
> > +#ifdef HAVE_SYS_CAPABILITY_H
> > + cap_t caps = NULL;
> > + int ret = -1;
> > +
> > + caps = cap_get_proc();
> > + if (!caps)
> > + goto out;
> > +
> > + ret = cap_clear_flag(caps, CAP_EFFECTIVE);
> > + if (ret)
> > + goto out;
> > +
> > + ret = cap_set_proc(caps);
> > + if (ret)
> > + goto out;
> > +
> > + fret = true;
> > +
> > +out:
> > + cap_free(caps);
> > +#endif
> > + return fret;
> > +}
> > +
> > +/* caps_down - raise all permitted caps */
>
> caps_up
Thanks for catching this!
>
> > +static int caps_up(void)
>
> [snip]
>
> > +static int print_r(int fd, const char *path)
>
> I saw build warnings:
>
> idmapped-mounts.c:554:12: warning: 'print_r' defined but not used [-Wunused-function]
> 554 | static int print_r(int fd, const char *path)
> | ^~~~~~~
>
> because DEBUG_TRACE is not defined.
Fixed.
Christian
prev parent reply other threads:[~2021-03-28 19:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-27 11:18 [PATCH v11 0/6] fstests: add idmapped mounts tests Christian Brauner
2021-03-27 11:18 ` [PATCH v11 1/6] generic/631: add test for detached mount propagation Christian Brauner
2021-03-27 11:18 ` [PATCH v11 3/6] common/rc: add _scratch_{u}mount_idmapped() helpers Christian Brauner
2021-03-28 18:35 ` Eryu Guan
2021-03-27 11:18 ` [PATCH v11 4/6] common/quota: move _qsetup() helper to common code Christian Brauner
2021-03-27 11:18 ` [PATCH v11 5/6] xfs/529: quotas and idmapped mounts Christian Brauner
2021-03-28 18:38 ` Eryu Guan
2021-03-27 11:18 ` [PATCH v11 6/6] xfs/530: quotas on " Christian Brauner
2021-03-27 11:28 ` [PATCH v11 0/6] fstests: add idmapped mounts tests Christian Brauner
[not found] ` <20210327111856.1211544-3-brauner@kernel.org>
2021-03-28 18:33 ` [PATCH v11 2/6] generic/632: add fstests for idmapped mounts Eryu Guan
2021-03-28 19:25 ` Christian Brauner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210328192532.jrw23brihkcrc6ok@wittgenstein \
--to=christian.brauner@ubuntu.com \
--cc=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=dhowells@redhat.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=guan@eryu.me \
--cc=hch@lst.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).