[Devel] [PATCH vz10 v2 1/2] selftests: ve: grant the directories the files sit in

[Konstantin Khorenko]

> The sysfs variant names nested leaves, kernel/vmcoreinfo and
> kernel/notes, while the proc one names files at the top of the
> filesystem. Both enforcement tests grant the leaves and nothing else,
> so in the sysfs case the lookup stops at /sys/kernel, which a fresh VE
> cannot traverse, and every open answers ENOENT:
> 
>   not ok 14 ve_perms.sysfs.enforce_allows
>   not ok 15 ve_perms.sysfs.enforce_denies
> 
> Grant x on each directory along the way, taken from the path the
> variant already gives. A file at the top of the filesystem has none
> and needs nothing, so the proc variant is unaffected.
> 
> The suite then passes 16 of 16, where it passed 14 before.

This line is redundant, please remove.
In general, please try to avoid numbers in commit messages which (numbers) will
change soon - this time it's enough just to add one more test and that's it.

> 
> Fixes: 303ce49b7446 ("selftests/ve: add ve_perms test for proc and sysfs")
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-144955
> 
> Feature: fix vz selftests
> Signed-off-by: Eva Kurchatova <eva.kurchatova at virtuozzo.com>
>
> diff --git a/tools/testing/selftests/ve/ve_perms_test.c b/tools/testing/selftests/ve/ve_perms_test.c
> index 25ffbd42c3802..2b542dc301394 100644
> --- a/tools/testing/selftests/ve/ve_perms_test.c
> +++ b/tools/testing/selftests/ve/ve_perms_test.c
> @@ -348,6 +348,37 @@ TEST_F(ve_perms, write_parser)
>   * read, and granting x on the directory lets it be traversed (the lookup then
>   * fails ENOENT on the missing child, not EACCES).
>   */

Those helpers were inserted between a test comment and the test itself.

> +/*
> + * A leaf is only reachable once every directory above it can be traversed.
> + * The variants name their files relative to the mount point, so grant x on
> + * each directory along the way; a file at the top of the filesystem has
> + * none and needs nothing.
> + */
> +static int allow_path(int cgv2_fd, const char *path, const char *file)
> +{
> +	char dir[240];	/* set_entry() writes "<dir> <mask>" into 256 */

The 240 is tied to the 256 inside set_entry() by a comment only, and
set_entry() itself still truncates silently: a long entry is written to
the cgroup file cut short instead of failing. The file already has the
idiom for this in ve_open_rel(), which checks the snprintf() result
against sizeof(). Apply the same in set_entry() and the caller no longer
needs its own length arithmetic:

         static int set_entry(int cgv2_fd, const char *path, const char *entry,
                             const char *mask)
         {
        -       char line[256];
        +       char line[PATH_MAX];

        -       snprintf(line, sizeof(line), "%s %s", entry, mask);
        +       if (snprintf(line, sizeof(line), "%s %s", entry, mask) >=
        +           (int)sizeof(line))
        +               return -1;
                return write_file_at(cgv2_fd, path, line);
         }

         static int allow_path(int cgv2_fd, const char *path, const char *file)
         {
        -       char dir[240];  /* set_entry() writes "<dir> <mask>" into 256 */
        +       char dir[PATH_MAX];
                const char *sep;

                for (sep = strchr(file, '/'); sep; sep = strchr(sep + 1, '/')) {
        -               if (sep - file >= (long)sizeof(dir))
        -                       return -1;
        -               snprintf(dir, sizeof(dir), "%.*s", (int)(sep - file), file);
        +               if (snprintf(dir, sizeof(dir), "%.*s", (int)(sep - file),
        +                            file) >= (int)sizeof(dir))
        +                       return -1;
                        if (set_entry(cgv2_fd, path, dir, "x") < 0)
                                return -1;
                }
                return 0;
         }

The set_entry() part fixes a pre-existing function, so it can go in its
own commit if you prefer, but it is small enough to stay here.

-- 
Konstantin Khorenko <khorenko at virtuozzo.com>