<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks, and fair point.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
First the bound: the walk in proc_perms_start is over the registered proc tree</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
(the proc_dir_entry nodes under proc_root), not the per task /proc/<pid> dirs, so</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
it does not grow with the number of processes. Per node it is cheap too,</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
proc_perms_shown returns right away when the node has no map, which is the common</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
case, and kmapset_lookup only runs on nodes that have one and walks just that</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
node's own entries, sorted so it stops early. It is not a scan of all keys.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I did not expect a container to read ve.proc_permissions at all, these are host</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
side cgroup control files, so I expected only the host to trigger this walk and</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
thought the atomic hold was fine. But while digging into it I discovered that a</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
container can read its own ve.proc_permissions and trigger the same walk, and the</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
existing ve.sysfs_permissions is the same. There is already a task to hide these ve</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
cgroup files inside containers, https://virtuozzo.atlassian.net/browse/VSTOR-120574,</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
which would take care of it. I discussed this with Pavel and we agreed to note it</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
as a known limitation for now.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
If you think the atomic section is a problem even when only the host can reach it, I</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
am happy to rewrite proc_perms_start and proc_perms_next to drop and retake</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
proc_subdir_lock on each step, the way proc_readdir_de does, so the held section is</div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
one node at a time.</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Vladimir Riabchun <vladimir.riabchun@virtuozzo.com><br>
<b>Sent:</b> Monday, June 29, 2026 6:21 PM<br>
<b>To:</b> Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>; Konstantin Khorenko <khorenko@virtuozzo.com>; Pavel Tikhomirov <ptikhomirov@virtuozzo.com><br>
<b>Cc:</b> devel@openvz.org <devel@openvz.org><br>
<b>Subject:</b> Re: [Devel] [PATCH vz10 0/7] per-VE ve.proc_permissions and sysfs permission fixes</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText"><br>
<br>
On 6/28/26 11:25, Mirian Shilakadze wrote:<br>
> This series adds ve.proc_permissions, the procfs counterpart of<br>
> ve.sysfs_permissions. It is a per-VE allowlist of /proc paths that the host<br>
> exposes to a container, keyed per VE so the single shared proc tree gives<br>
> per-VE answers.<br>
> <br>
> The motivation is GPU support in containers. A containerized GPU workload<br>
> needs a few host /proc files visible (the nvidia entries it probes), and<br>
> ve.proc_permissions exposes them through a generic per-VE allowlist rather<br>
> than an nvidia specific passthrough.<br>
> <br>
> While implementing it I found several pre-existing defects in the shared<br>
> sysfs and kernfs per-VE permission path, so the series is fix-first. Reading<br>
> ve.sysfs_permissions under load already panicked the host on a stock kernel<br>
> (NULL deref in kmapset_lookup), which the early patches fix before the procfs<br>
> work builds on the same code. Of these defects the ve_perms_map<br>
> use-after-free in patch 5 (the __rcu annotation) was found by code analysis.<br>
> The rest surfaced through testing, the NULL deref and the wrong rwsem from the<br>
> runtime crash and lockdep, and the rcu-list walks from PROVE_RCU_LIST.<br>
> <br>
> Layout:<br>
> 1: lib/kmapset annotates the kmapset_lookup rcu-list walk so it is honest<br>
> under CONFIG_PROVE_RCU_LIST.<br>
> 2 to 5: fix the kernfs seq read and the VFS readers, skip a NULL map, lock<br>
> the tree that is actually walked, take rcu_read_lock around the kmapset<br>
> lookup, and mark ve_perms_map __rcu to close a use-after-free.<br>
> 6: factors the filesystem agnostic core into fs/ve_perms.c with no<br>
> functional change beyond an rcu_assign_pointer publish.<br>
> 7: adds the procfs feature on top.<br>
> <br>
> Testing: Built and booted a debug kernel with KASAN, kmemleak, lockdep and<br>
> PROVE_RCU_LIST. Ran concurrent reader, writer and teardown stress on both<br>
> ve.sysfs_permissions and ve.proc_permissions, including in-container /proc<br>
> and sysfs access and container start and stop. The original NULL deref<br>
> reproduces on a stock kernel and no longer crashes with this series. No KASAN<br>
> use-after-free, no kmemleak leak, and no rcu-list or lockdep splat in the<br>
> ve_perms paths. gcov line coverage of the four touched files reached 93 to<br>
> 99 percent (fs/kernfs/ve.c 99, fs/proc/ve.c 97, fs/ve_perms.c 95,<br>
> lib/kmapset.c 93), the remainder being inlined fortify checks, error and<br>
> boot-only init paths. Per-VE correctness was checked separately on both<br>
> filesystems. A path becomes visible and readable inside a container only<br>
> after it is added to that VE allowlist, access is revoked when it is<br>
> removed, the host is unaffected, and the entry never leaks to another VE.<br>
<br>
Just a brief question: it seems to me that /proc can have a very big number<br>
of entries.<br>
<br>
When we start in proc_perms_start, we take proc_subdir_lock, which is atomic.<br>
For each entry we do kmapset_lookup, where we iterate over all added keys(if<br>
I got correctly). And in general we do a big number of kmapset operations for<br>
each entry.<br>
<br>
Won't we get any performance issues, if we have, for example, huge amount of<br>
processes and several CTs? Such long atomic sections could lead to lockups.<br>
<br>
Other than that the code looks good, didn't find any obvious bugs.<br>
<br>
> <br>
> Mirian Shilakadze (7):<br>
> lib/kmapset: annotate the kmapset_lookup rcu-list walk with the held<br>
> lock<br>
> fs/kernfs, ve: skip NULL ve_perms_map in kernfs_perms_shown<br>
> fs/kernfs, ve: lock the walked tree rwsem in kernfs_perms_start<br>
> fs/kernfs, ve: take rcu_read_lock around the ve_perms kmapset lookup<br>
> fs/kernfs, ve: fix ve_perms_map use-after-free, annotate it __rcu<br>
> fs: factor per-VE permission core into ve_perms helpers<br>
> fs/proc, ve: add per-VE ve.proc_permissions<br>
> <br>
> fs/Makefile | 1 +<br>
> fs/kernfs/ve.c | 167 ++++++++----------<br>
> fs/proc/Makefile | 1 +<br>
> fs/proc/generic.c | 48 +++++-<br>
> fs/proc/inode.c | 2 +<br>
> fs/proc/internal.h | 25 +++<br>
> fs/proc/root.c | 1 +<br>
> fs/proc/ve.c | 345 ++++++++++++++++++++++++++++++++++++++<br>
> fs/sysfs/ve.c | 2 +-<br>
> fs/ve_perms.c | 136 +++++++++++++++<br>
> include/linux/kernfs-ve.h | 2 +-<br>
> include/linux/kernfs.h | 2 +-<br>
> include/linux/ve-perms.h | 28 ++++<br>
> include/linux/ve.h | 1 +<br>
> kernel/ve/ve.c | 7 +<br>
> lib/kmapset.c | 3 +-<br>
> 16 files changed, 665 insertions(+), 106 deletions(-)<br>
> create mode 100644 fs/proc/ve.c<br>
> create mode 100644 fs/ve_perms.c<br>
> create mode 100644 include/linux/ve-perms.h<br>
> <br>
> --<br>
> 2.43.0<br>
> <br>
> _______________________________________________<br>
> Devel mailing list<br>
> Devel@openvz.org<br>
> <a href="https://lists.openvz.org/mailman/listinfo/devel">https://lists.openvz.org/mailman/listinfo/devel</a><br>
<br>
-- <br>
Best regards, Riabchun Vladimir<br>
Linux Kernel Developer, Virtuozzo<br>
<br>
</div>
</span></font></div>
</body>
</html>