On Fri, Apr 23, 2021 at 01:15:39PM +0200, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@xxxxxxxxxx>
>
> Add a test to verify that setting a v3 fscap that is valid in an
> ancestor user namespace works.
The subject is not clear which test it updates, I can only know it's
generic/633 that calls idmapped-mounts binary to do the test.
>
> Cc: fstests@xxxxxxxxxxxxxxx
> Cc: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
> ---
> src/idmapped-mounts/idmapped-mounts.c | 56 +++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> index 870a8fe7..4e3252ca 100644
> --- a/src/idmapped-mounts/idmapped-mounts.c
> +++ b/src/idmapped-mounts/idmapped-mounts.c
> @@ -3193,6 +3193,62 @@ static int fscaps_idmapped_mounts_in_userns(void)
> goto out;
> }
>
> + /*
> + * Verify we can set an v3 fscap for real root this was regressed at
> + * some point. Make sure this doesn't happen again!
> + */
We usually don't add new test cases to existing tests, as that may
introduce new failures and let people think there's a regression, then
find out it's the new case introduced the failure.
But this test was just merged last week, and the test is closely related
to existing cases and could re-use the test framework/setups, so I think
it's fine to add this case.
But as above comment said, this new cases is targeted to a regression
happened previously, I think it'd be better to put it in a seperate test
function, not folded into an existing test function.
And is there a commit that fixed the mentioned regression? Reference it
in the comments would help people find the correct fix, if there's any.
Thanks,
Eryu
> + if (fremovexattr(file1_fd, "security.capability")) {
> + log_stderr("failure: fremovexattr");
> + goto out;
> + }
> + if (expected_dummy_vfs_caps_uid(file1_fd, -1)) {
> + log_stderr("failure: expected_dummy_vfs_caps_uid");
> + goto out;
> + }
> + if (errno != ENODATA) {
> + log_stderr("failure: errno");
> + goto out;
> + }
> +
> + pid = fork();
> + if (pid < 0) {
> + log_stderr("failure: fork");
> + goto out;
> + }
> + if (pid == 0) {
> + if (!switch_userns(attr.userns_fd, 0, 0, false))
> + die("failure: switch_userns");
> +
> + if (expected_dummy_vfs_caps_uid(file1_fd2, -1))
> + die("failure: expected_dummy_vfs_caps_uid");
> + if (errno != ENODATA)
> + die("failure: errno");
> +
> + if (set_dummy_vfs_caps(file1_fd2, 0, 0))
> + die("failure: set_dummy_vfs_caps");
> +
> + if (!expected_dummy_vfs_caps_uid(file1_fd2, 0))
> + die("failure: expected_dummy_vfs_caps_uid");
> +
> + if (!expected_dummy_vfs_caps_uid(file1_fd, 0) && errno != EOVERFLOW)
> + die("failure: expected_dummy_vfs_caps_uid");
> +
> + exit(EXIT_SUCCESS);
> + }
> +
> + if (wait_for_pid(pid))
> + goto out;
> +
> + if (!expected_dummy_vfs_caps_uid(file1_fd2, 10000)) {
> + log_stderr("failure: expected_dummy_vfs_caps_uid");
> + goto out;
> + }
> +
> + if (!expected_dummy_vfs_caps_uid(file1_fd, 0)) {
> + log_stderr("failure: expected_dummy_vfs_caps_uid");
> + goto out;
> + }
> +
> fret = 0;
> log_debug("Ran test");
> out:
>
> base-commit: 15510d3a208187e234333f7974580786d54d52dc
> --
> 2.27.0
