Re: [Devel] [PATCH hci-8.1] selftests: ve: grant the root directory as well

[Konstantin Khorenko <khorenko@virtuozzo.com>]

Hi Eva,

The fix itself is right. On sysfs a fresh VE cannot traverse /sys
without a grant on the root. On proc the "/ x" write is refused with
EINVAL, and the proc root needs no grant. Remarks inline, three on the
message and one minor on the code.

On 11/2/26 16:08, Eva Kurchatova wrote:
> allow_path() grants every directory above a file except the root, so a
> fresh VE can reach the file only if the default permissions already let
> it traverse the root, as vzctl sets them at boot. Without that,
> ve_perms.sysfs.enforce_allows fails.

ve_perms.sysfs.enforce_denies is affected as well, the other way round:
it passed for the wrong reason. Its "granted w only" and "directory
granted r only" checks expect EACCES. On such a host they got it from the
root, not from the missing r and x bits. This patch is what makes those
checks test something, worth one sentence here.

> Grant the root too, proc refuses an
> entry for its root with EINVAL, as its root is always reachable.

Two sentences joined with commas. Also, "as its root is always
reachable" reads as the reason proc refuses the entry. The real reason
is in proc_perms_set(): an empty path walks to proc_root and is rejected
as invalid. Reachability is why the test may ignore that error. Maybe:

  Grant the root too. proc refuses an entry for its root with EINVAL
  and needs none: the proc root inode has no per-VE permission check.

> Fixes: ce7800c78ae9 ("selftests: ve: grant the directories the files sit in")
> Feature: fix selftests

The Jira URL line is missing. ce7800c78ae9 carries VSTOR-144955. If this
is the same ticket, please add

  https://virtuozzo.atlassian.net/browse/VSTOR-144955

> @@ -351,6 +351,8 @@ static int allow_path(int cgv2_fd, const char *path, const char *file)
>  	char dir[PATH_MAX];
>  	const char *sep;
>
> +	if (set_entry(cgv2_fd, path, "/", "x") < 0 && errno != EINVAL)
> +		return -1;

allow_paths() calls allow_path() once per file, so the root is granted NR_FILES
times. Unlike the intermediate directories, the root is the same for every file,
so it could go into allow_paths(), once, before the loop:

	if (set_entry(cgv2_fd, path, "/", "x") < 0 && errno != EINVAL)
		return -1;
	for (i = 0; i < NR_FILES; i++)
		if (allow_path(cgv2_fd, path, files[i]) < 0)
			return -1;

--
Konstantin

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel