<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 style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Yes we need it, for the .permission path.</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);">
A cached inode holds a pde reference that outlives remove_proc_entry and rundown</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_evict_inode does not drop it, only proc_free_inode does). proc_ve_permission,</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
via the .permission op, reads de->ve_perms_map as a lock free rcu reader. So if 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);">
runs on that cached inode after proc_put_ve_perms and the nullify is not there, 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);">
field still points at the old map. kmapset_put drops our reference and, if this pde</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
held the last one, frees it via kfree_rcu. Once the grace period passes the map is</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
freed but the field still points at it, so a later reader loads a dangling pointer</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
and reads freed memory. The nullify publishes NULL before the free, the standard rcu</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
remove then free, so the reader sees NULL and returns -EACCES, which is also correct</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
for a removed entry.</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);">
We cannot drop the map at the pde's final free the way sysfs does, because proc can</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
free the pde from an rcu callback. When a cached inode holds the last reference, 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);">
final pde_put runs from proc_free_inode (.free_inode), which is dispatched from</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
call_rcu (destroy_inode -> call_rcu -> i_callback -> free_inode), so it runs as an</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
rcu callback and must not sleep. kmapset_put takes a mutex on the last reference</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
(kref_put_mutex on set->mutex), so it can sleep and cannot run there. That is why</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 drops the map in process context from proc_entry_rundown at removal, before 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);">
pde is freed and while it is still reachable, which is what the nullify covers.</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);">
sysfs does not need this. kernfs_put_ve_perms runs only from kernfs_put once the kn</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
refcount hits zero, before the kn is rcu freed, and the kernfs inode holds a count</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
reference on the kn, so a .permission reader keeps the count above zero and cannot</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
race the drop. The proc tree readers (proc_lookup_de, proc_readdir_de 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);">
ve.proc_permissions seq read) never reach a removed pde either, because</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
remove_proc_entry unlinks it under proc_subdir_lock before rundown. Only the</div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
.permission path, through a cached inode, can reach the pde after the put.</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> Pavel Tikhomirov <ptikhomirov@virtuozzo.com><br>
<b>Sent:</b> Tuesday, June 30, 2026 4:41 PM<br>
<b>To:</b> Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>; Konstantin Khorenko <khorenko@virtuozzo.com><br>
<b>Cc:</b> devel@openvz.org <devel@openvz.org>; den@openvz.org <den@openvz.org><br>
<b>Subject:</b> Re: [PATCH vz10 7/7] fs/proc, ve: add per-VE ve.proc_permissions</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:26, Mirian Shilakadze wrote:<br>
> Add a per-VE allowlist of /proc paths exposed through the cgroup file<br>
> ve.proc_permissions, the procfs counterpart of ve.sysfs_permissions.<br>
> Each proc_dir_entry gains a kmapset map keyed by VE (proc_perms_key on<br>
> ve_struct), so the single shared proc tree yields per-VE answers. The<br>
> filesystem agnostic leaf logic is reused from fs/ve_perms.c, this commit<br>
> adds the proc tree walk, the locking, and the VFS hooks: visibility in<br>
> proc_lookup_de/proc_readdir_de and a .permission inode op.<br>
> <br>
> Paths are written relative to the proc root like sysfs, path mask where<br>
> mask is r/w/x or - to remove. The host (ve0) is unaffected and an empty<br>
> list exposes nothing extra. The lock-free readers load the map under rcu<br>
> against the writer's copy-on-write swap, and the seq read is serialised<br>
> against the writer by proc_perms_mutex.<br>
> <br>
> Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com><br>
> ---<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>
> include/linux/ve.h | 1 +<br>
> kernel/ve/ve.c | 7 +<br>
> 8 files changed, 423 insertions(+), 7 deletions(-)<br>
> create mode 100644 fs/proc/ve.c<br>
> <br>
> diff --git a/fs/proc/Makefile b/fs/proc/Makefile<br>
> index 7b4db9c56e6a..61a999c03663 100644<br>
> --- a/fs/proc/Makefile<br>
> +++ b/fs/proc/Makefile<br>
> @@ -11,6 +11,7 @@ proc-$(CONFIG_MMU) := task_mmu.o<br>
> <br>
> proc-y += inode.o root.o base.o generic.o array.o \<br>
> fd.o<br>
> +proc-$(CONFIG_VE) += ve.o<br>
> proc-$(CONFIG_TTY) += proc_tty.o<br>
> proc-y += cmdline.o<br>
> proc-y += consoles.o<br>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c<br>
> index e8fd7c2d1c3a..791c38c49a86 100644<br>
> --- a/fs/proc/generic.c<br>
> +++ b/fs/proc/generic.c<br>
> @@ -30,7 +30,7 @@<br>
> <br>
> #include "internal.h"<br>
> <br>
> -static DEFINE_RWLOCK(proc_subdir_lock);<br>
> +DEFINE_RWLOCK(proc_subdir_lock);<br>
> <br>
> struct kmem_cache *proc_dir_entry_cache __ro_after_init;<br>
> <br>
> @@ -65,9 +65,9 @@ static struct proc_dir_entry *pde_subdir_next(struct proc_dir_entry *dir)<br>
> subdir_node);<br>
> }<br>
> <br>
> -static struct proc_dir_entry *pde_subdir_find(struct proc_dir_entry *dir,<br>
> - const char *name,<br>
> - unsigned int len)<br>
> +struct proc_dir_entry *pde_subdir_find(struct proc_dir_entry *dir,<br>
> + const char *name,<br>
> + unsigned int len)<br>
> {<br>
> struct rb_node *node = dir->subdir.rb_node;<br>
> <br>
> @@ -120,6 +120,31 @@ static bool proc_in_container(struct super_block *sb)<br>
> return !ve_is_super(get_exec_env());<br>
> }<br>
> <br>
> +/* Visible to the current VE: globally published (S_ISVTX) or per-VE allowed. */<br>
> +static bool pde_visible_to_ve(struct proc_dir_entry *de)<br>
> +{<br>
> + return (de->mode & S_ISVTX) || proc_d_visible(de);<br>
> +}<br>
> +<br>
> +#ifdef CONFIG_VE<br>
> +static int proc_iop_permission(struct mnt_idmap *idmap, struct inode *inode,<br>
> + int mask)<br>
> +{<br>
> + struct proc_dir_entry *de = PDE(inode);<br>
> + int ret = 0;<br>
> +<br>
> + /*<br>
> + * Runs safely during rcu-walk: proc_ve_permission() is a lockless rcu<br>
> + * kmapset lookup and generic_permission() copes with rcu-walk on its<br>
> + * own, so MAY_NOT_BLOCK needs no special handling here.<br>
> + */<br>
> + if (proc_in_container(inode->i_sb) && !(de->mode & S_ISVTX))<br>
> + ret = proc_ve_permission(de, mask);<br>
> +<br>
> + return ret ? ret : generic_permission(idmap, inode, mask);<br>
> +}<br>
> +#endif<br>
> +<br>
> static int proc_notify_change(struct mnt_idmap *idmap,<br>
> struct dentry *dentry, struct iattr *iattr)<br>
> {<br>
> @@ -167,6 +192,9 @@ static int proc_getattr(struct mnt_idmap *idmap,<br>
> <br>
> static const struct inode_operations proc_file_inode_operations = {<br>
> .setattr = proc_notify_change,<br>
> +#ifdef CONFIG_VE<br>
> + .permission = proc_iop_permission,<br>
> +#endif<br>
> };<br>
> <br>
> /*<br>
> @@ -264,7 +292,7 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry,<br>
> read_lock(&proc_subdir_lock);<br>
> de = pde_subdir_find(de, dentry->d_name.name, dentry->d_name.len);<br>
> if (de) {<br>
> - if (in_container && !(de->mode & S_ISVTX)) {<br>
> + if (in_container && !pde_visible_to_ve(de)) {<br>
> read_unlock(&proc_subdir_lock);<br>
> return ERR_PTR(-ENOENT);<br>
> }<br>
> @@ -317,7 +345,7 @@ int proc_readdir_de(struct file *file, struct dir_context *ctx,<br>
> read_unlock(&proc_subdir_lock);<br>
> return 0;<br>
> }<br>
> - if (!in_container || (de->mode & S_ISVTX)) {<br>
> + if (!in_container || pde_visible_to_ve(de)) {<br>
> if (!i)<br>
> break;<br>
> i--;<br>
> @@ -328,7 +356,7 @@ int proc_readdir_de(struct file *file, struct dir_context *ctx,<br>
> do {<br>
> struct proc_dir_entry *next;<br>
> <br>
> - if (in_container && !(de->mode & S_ISVTX)) {<br>
> + if (in_container && !pde_visible_to_ve(de)) {<br>
> de = pde_subdir_next(de);<br>
> continue;<br>
> }<br>
> @@ -389,6 +417,9 @@ static const struct inode_operations proc_dir_inode_operations = {<br>
> .lookup = proc_lookup,<br>
> .getattr = proc_getattr,<br>
> .setattr = proc_notify_change,<br>
> +#ifdef CONFIG_VE<br>
> + .permission = proc_iop_permission,<br>
> +#endif<br>
> };<br>
> <br>
> /* returns the registered entry, or frees dp and returns NULL on failure */<br>
> @@ -413,6 +444,7 @@ struct proc_dir_entry *proc_register(struct proc_dir_entry *dir,<br>
> out_free_inum:<br>
> proc_free_inum(dp->low_ino);<br>
> out_free_entry:<br>
> + proc_put_ve_perms(dp);<br>
> pde_free(dp);<br>
> return NULL;<br>
> }<br>
> @@ -471,6 +503,7 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent,<br>
> ent->nlink = nlink;<br>
> ent->subdir = RB_ROOT;<br>
> refcount_set(&ent->refcnt, 1);<br>
> + proc_get_ve_perms(ent);<br>
> spin_lock_init(&ent->pde_unload_lock);<br>
> INIT_LIST_HEAD(&ent->pde_openers);<br>
> proc_set_user(ent, (*parent)->uid, (*parent)->gid);<br>
> @@ -498,6 +531,7 @@ struct proc_dir_entry *proc_symlink_mode(const char *name, umode_t mode,<br>
> ent->proc_iops = &proc_link_inode_operations;<br>
> ent = proc_register(parent, ent);<br>
> } else {<br>
> + proc_put_ve_perms(ent);<br>
> pde_free(ent);<br>
> ent = NULL;<br>
> }<br>
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c<br>
> index 5d1a75408aa4..ac943a9768f4 100644<br>
> --- a/fs/proc/inode.c<br>
> +++ b/fs/proc/inode.c<br>
> @@ -272,6 +272,8 @@ void proc_entry_rundown(struct proc_dir_entry *de)<br>
> spin_lock(&de->pde_unload_lock);<br>
> }<br>
> spin_unlock(&de->pde_unload_lock);<br>
> +<br>
> + proc_put_ve_perms(de);<br>
> }<br>
> <br>
> static loff_t proc_reg_llseek(struct file *file, loff_t offset, int whence)<br>
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h<br>
> index 77a517f91821..4a28da7d5dee 100644<br>
> --- a/fs/proc/internal.h<br>
> +++ b/fs/proc/internal.h<br>
> @@ -17,6 +17,7 @@<br>
> <br>
> struct ctl_table_header;<br>
> struct mempolicy;<br>
> +struct kmapset_map;<br>
> <br>
> /*<br>
> * This is not completely implemented yet. The idea is to<br>
> @@ -64,6 +65,9 @@ struct proc_dir_entry {<br>
> umode_t mode;<br>
> u8 flags;<br>
> u8 namelen;<br>
> +#ifdef CONFIG_VE<br>
> + struct kmapset_map __rcu *ve_perms_map; /* per-VE r/w/x mask for this node */<br>
> +#endif<br>
> char inline_name[];<br>
> } __randomize_layout;<br>
> <br>
> @@ -102,6 +106,27 @@ static inline bool pde_has_proc_compat_ioctl(const struct proc_dir_entry *pde)<br>
> extern struct kmem_cache *proc_dir_entry_cache;<br>
> void pde_free(struct proc_dir_entry *pde);<br>
> <br>
> +extern rwlock_t proc_subdir_lock;<br>
> +struct proc_dir_entry *pde_subdir_find(struct proc_dir_entry *dir,<br>
> + const char *name, unsigned int len);<br>
> +<br>
> +#ifdef CONFIG_VE<br>
> +void proc_init_ve_perms(void);<br>
> +void proc_get_ve_perms(struct proc_dir_entry *de);<br>
> +void proc_put_ve_perms(struct proc_dir_entry *de);<br>
> +bool proc_d_visible(struct proc_dir_entry *de);<br>
> +int proc_ve_permission(struct proc_dir_entry *de, int mask);<br>
> +#else<br>
> +static inline void proc_init_ve_perms(void) { }<br>
> +static inline void proc_get_ve_perms(struct proc_dir_entry *de) { }<br>
> +static inline void proc_put_ve_perms(struct proc_dir_entry *de) { }<br>
> +static inline bool proc_d_visible(struct proc_dir_entry *de) { return false; }<br>
> +static inline int proc_ve_permission(struct proc_dir_entry *de, int mask)<br>
> +{<br>
> + return 0;<br>
> +}<br>
> +#endif<br>
> +<br>
> union proc_op {<br>
> int (*proc_get_link)(struct dentry *, struct path *);<br>
> int (*proc_show)(struct seq_file *m,<br>
> diff --git a/fs/proc/root.c b/fs/proc/root.c<br>
> index 3f61de56ffff..9e0c5bf87602 100644<br>
> --- a/fs/proc/root.c<br>
> +++ b/fs/proc/root.c<br>
> @@ -297,6 +297,7 @@ static struct file_system_type proc_fs_type = {<br>
> void __init proc_root_init(void)<br>
> {<br>
> proc_init_kmemcache();<br>
> + proc_init_ve_perms();<br>
> set_proc_pid_nlink();<br>
> proc_self_init();<br>
> proc_thread_self_init();<br>
> diff --git a/fs/proc/ve.c b/fs/proc/ve.c<br>
> new file mode 100644<br>
> index 000000000000..10106c2a4e53<br>
> --- /dev/null<br>
> +++ b/fs/proc/ve.c<br>
> @@ -0,0 +1,345 @@<br>
> +// SPDX-License-Identifier: GPL-2.0<br>
> +/*<br>
> + * Per-VE /proc permissions (ve.proc_permissions), the procfs counterpart of<br>
> + * the sysfs ve.sysfs_permissions mechanism. Each proc_dir_entry carries a<br>
> + * kmapset map keyed by VE, so the single shared proc tree gives per-VE<br>
> + * answers. The filesystem agnostic leaf logic lives in fs/ve_perms.c, this<br>
> + * file owns the proc tree walk and the locking.<br>
> + *<br>
> + * Copyright (c) 2026 Virtuozzo International GmbH. All rights reserved.<br>
> + */<br>
> +<br>
> +#include <linux/module.h><br>
> +#include <linux/errno.h><br>
> +#include <linux/slab.h><br>
> +#include <linux/string.h><br>
> +#include <linux/rbtree.h><br>
> +#include <linux/rcupdate.h><br>
> +#include <linux/seq_file.h><br>
> +#include <linux/fs.h><br>
> +#include <linux/cgroup.h><br>
> +#include <linux/ve.h><br>
> +#include <linux/kmapset.h><br>
> +#include <linux/ve-perms.h><br>
> +#include <linux/proc_fs.h><br>
> +<br>
> +#include "internal.h"<br>
> +<br>
> +struct kmapset_set proc_ve_perms_set;<br>
> +<br>
> +static bool proc_ve_perms_inited;<br>
> +<br>
> +static DEFINE_MUTEX(proc_perms_mutex);<br>
> +<br>
> +void __init proc_init_ve_perms(void)<br>
> +{<br>
> + struct kmapset_map *map;<br>
> +<br>
> + kmapset_init_set(&proc_ve_perms_set);<br>
> + map = kmapset_new(&proc_ve_perms_set);<br>
> + if (map)<br>
> + RCU_INIT_POINTER(proc_root.ve_perms_map, kmapset_commit(map));<br>
> + proc_ve_perms_inited = true;<br>
> +}<br>
> +<br>
> +void proc_get_ve_perms(struct proc_dir_entry *de)<br>
> +{<br>
> + struct kmapset_map *map;<br>
> +<br>
> + if (!proc_ve_perms_inited)<br>
> + return;<br>
> +<br>
> + map = kmapset_new(&proc_ve_perms_set);<br>
> + if (map)<br>
> + rcu_assign_pointer(de->ve_perms_map, kmapset_commit(map));<br>
> + else<br>
> + pr_warn_once("proc: no ve_perms_map for %s, hidden from containers\n",<br>
> + de->name);<br>
> +}<br>
> +<br>
> +/*<br>
> + * Drop the node's permission map. kmapset_put() can sleep (it takes the<br>
> + * kmapset set mutex on the last reference), so every caller must be in process<br>
> + * context. Registered entries drop the map from proc_entry_rundown() when they<br>
> + * are removed. Entries that never reach the tree drop it on their creation error<br>
> + * path (proc_register(), proc_symlink_mode()). pde_free() therefore never<br>
> + * touches the map and stays safe to run from the .free_inode RCU callback, which<br>
> + * is atomic. The mutex here serialises against a concurrent proc_perms_set() on<br>
> + * a registered entry. The NULL fast path skips it when there is nothing to drop.<br>
> + */<br>
> +void proc_put_ve_perms(struct proc_dir_entry *de)<br>
> +{<br>
> + struct kmapset_map *map;<br>
> +<br>
> + /* Atomic-safe fast path: already dropped at rundown, or never set. */<br>
> + if (!rcu_access_pointer(de->ve_perms_map))<br>
> + return;<br>
> +<br>
> + /* Serialise against a concurrent proc_perms_set() on this entry. */<br>
> + mutex_lock(&proc_perms_mutex);<br>
> + map = rcu_dereference_protected(de->ve_perms_map,<br>
> + lockdep_is_held(&proc_perms_mutex));<br>
> + rcu_assign_pointer(de->ve_perms_map, NULL);<br>
<br>
I think that is exactly the thing why you need the second patch.<br>
Do we really need to nulify this? Can we somehow guarantee that de<br>
is not used after this and thus stale ve_perms_map is not accessed?<br>
<br>
Or alternatively should we also nulify ve_perms_map for sysfs too?<br>
<br>
> + mutex_unlock(&proc_perms_mutex);<br>
> + kmapset_put(map);<br>
> +}<br>
> +<br>
> +bool proc_d_visible(struct proc_dir_entry *de)<br>
> +{<br>
> + struct ve_struct *ve = get_exec_env();<br>
> + struct kmapset_map *map;<br>
> + bool visible;<br>
> +<br>
> + if (ve_is_super(ve))<br>
> + return true;<br>
> +<br>
> + /*<br>
> + * proc_perms_set() can swap this map pointer concurrently and free the<br>
> + * old map through kfree_rcu(). Hold rcu across both the load and the<br>
> + * lookup so the map cannot be freed under us.<br>
> + */<br>
> + rcu_read_lock();<br>
> + map = rcu_dereference(de->ve_perms_map);<br>
> + visible = map && ve_perms_visible(map, &ve->proc_perms_key);<br>
> + rcu_read_unlock();<br>
> + return visible;<br>
> +}<br>
> +<br>
> +int proc_ve_permission(struct proc_dir_entry *de, int mask)<br>
> +{<br>
> + struct ve_struct *ve = get_exec_env();<br>
> + struct kmapset_map *map;<br>
> + int ret;<br>
> +<br>
> + if (ve_is_super(ve))<br>
> + return 0;<br>
> +<br>
> + rcu_read_lock();<br>
> + map = rcu_dereference(de->ve_perms_map);<br>
> + ret = map ? ve_perms_check(map, &ve->proc_perms_key, mask) : -EACCES;<br>
> + rcu_read_unlock();<br>
> + return ret;<br>
> +}<br>
> +<br>
> +static int proc_perms_set(char *path, struct ve_struct *ve, int mask)<br>
> +{<br>
> + struct proc_dir_entry *de, *nde;<br>
> + char *sep = path, *dname;<br>
> + int ret = 0;<br>
> +<br>
> + read_lock(&proc_subdir_lock);<br>
> + de = &proc_root;<br>
> + pde_get(de);<br>
> + do {<br>
> + dname = sep;<br>
> +<br>
> + sep = strchr(sep, '/');<br>
> + if (sep)<br>
> + *sep++ = 0;<br>
> +<br>
> + if (!*dname)<br>
> + break;<br>
> +<br>
> + nde = pde_subdir_find(de, dname, strlen(dname));<br>
> + if (!nde) {<br>
> + read_unlock(&proc_subdir_lock);<br>
> + ret = -ENOENT;<br>
> + goto out;<br>
> + }<br>
> + pde_get(nde);<br>
> + pde_put(de);<br>
> + de = nde;<br>
> + } while (sep);<br>
> + read_unlock(&proc_subdir_lock);<br>
> +<br>
> + /* empty or leading-slash path walks to nothing, reject it */<br>
> + if (de == &proc_root) {<br>
> + ret = -EINVAL;<br>
> + goto out;<br>
> + }<br>
> +<br>
> + if (!rcu_access_pointer(de->ve_perms_map)) {<br>
> + ret = -EPERM;<br>
> + goto out;<br>
> + }<br>
> +<br>
> + ret = ve_perms_apply(&de->ve_perms_map, &ve->proc_perms_key,<br>
> + ve_is_super(ve), mask);<br>
> +out:<br>
> + pde_put(de);<br>
> + return ret;<br>
> +}<br>
> +<br>
> +static int proc_perms_line(struct ve_struct *ve, char *line)<br>
> +{<br>
> + int mask, ret;<br>
> +<br>
> + ret = ve_perms_parse(line, &mask);<br>
> + if (ret)<br>
> + return ret;<br>
> + return proc_perms_set(line, ve, mask);<br>
> +}<br>
> +<br>
> +static struct proc_dir_entry *proc_next_recursive(struct proc_dir_entry *de)<br>
> +{<br>
> + struct rb_node *node;<br>
> +<br>
> + node = rb_first(&de->subdir);<br>
> + if (node)<br>
> + return rb_entry(node, struct proc_dir_entry, subdir_node);<br>
> +<br>
> + while (de->parent != de) {<br>
> + node = rb_next(&de->subdir_node);<br>
> + if (node)<br>
> + return rb_entry(node, struct proc_dir_entry,<br>
> + subdir_node);<br>
> + de = de->parent;<br>
> + }<br>
> + return NULL;<br>
> +}<br>
> +<br>
> +static bool proc_perms_shown(struct ve_struct *ve, struct proc_dir_entry *de)<br>
> +{<br>
> + bool shown;<br>
> +<br>
> + if (!rcu_access_pointer(de->ve_perms_map))<br>
> + return false;<br>
> +<br>
> + /* ve_perms_shown calls kmapset_lookup, an rcu list walk, guard it. */<br>
> + rcu_read_lock();<br>
> + shown = ve_perms_shown(rcu_dereference(de->ve_perms_map),<br>
> + &ve->proc_perms_key, ve_is_super(ve));<br>
> + rcu_read_unlock();<br>
> + return shown;<br>
> +}<br>
> +<br>
> +static void *proc_perms_start(struct seq_file *m, loff_t *ppos)<br>
> + __acquires(&proc_subdir_lock)<br>
> +{<br>
> + struct ve_struct *ve = css_to_ve(seq_css(m));<br>
> + struct proc_dir_entry *de;<br>
> + loff_t pos = *ppos;<br>
> +<br>
> + mutex_lock(&proc_perms_mutex);<br>
> + read_lock(&proc_subdir_lock);<br>
> + for (de = &proc_root; de; de = proc_next_recursive(de)) {<br>
> + if (proc_perms_shown(ve, de) && !pos--)<br>
> + break;<br>
> + }<br>
> + return de;<br>
> +}<br>
> +<br>
> +static void *proc_perms_next(struct seq_file *m, void *v, loff_t *ppos)<br>
> +{<br>
> + struct ve_struct *ve = css_to_ve(seq_css(m));<br>
> + struct proc_dir_entry *de = v;<br>
> +<br>
> + (*ppos)++;<br>
> + while ((de = proc_next_recursive(de))) {<br>
> + if (proc_perms_shown(ve, de))<br>
> + break;<br>
> + }<br>
> + return de;<br>
> +}<br>
> +<br>
> +static void proc_perms_stop(struct seq_file *m, void *v)<br>
> + __releases(&proc_subdir_lock)<br>
> +{<br>
> + read_unlock(&proc_subdir_lock);<br>
> + mutex_unlock(&proc_perms_mutex);<br>
> +}<br>
> +<br>
> +static int proc_perms_show(struct seq_file *m, void *v)<br>
> +{<br>
> + struct ve_struct *ve = css_to_ve(seq_css(m));<br>
> + struct proc_dir_entry *de = v;<br>
> + struct kmapset_map *map;<br>
> + char *buf;<br>
> + size_t size, len, off;<br>
> + int mask;<br>
> +<br>
> + map = rcu_dereference_protected(de->ve_perms_map,<br>
> + lockdep_is_held(&proc_perms_mutex));<br>
> + if (ve_is_super(ve))<br>
> + mask = map->default_value;<br>
> + else<br>
> + mask = kmapset_get_value(map, &ve->proc_perms_key);<br>
> +<br>
> + size = seq_get_buf(m, &buf);<br>
> + if (size) {<br>
> + off = size;<br>
> + do {<br>
> + len = strlen(de->name);<br>
> + if (len >= off) {<br>
> + seq_commit(m, -1);<br>
> + return 0;<br>
> + }<br>
> + if (S_ISDIR(de->mode))<br>
> + buf[--off] = '/';<br>
> + off -= len;<br>
> + memcpy(buf + off, de->name, len);<br>
> + de = de->parent;<br>
> + } while (de && de != &proc_root);<br>
> + memmove(buf, buf + off, size - off);<br>
> + seq_commit(m, size - off);<br>
> + }<br>
> +<br>
> + ve_perms_emit(m, mask);<br>
> + return 0;<br>
> +}<br>
> +<br>
> +static ssize_t proc_perms_write(struct kernfs_open_file *of, char *buf,<br>
> + size_t nbytes, loff_t off)<br>
> +{<br>
> + struct ve_struct *ve = css_to_ve(of_css(of));<br>
> + char *line, *next = buf;<br>
> + int ret = -EINVAL;<br>
> +<br>
> + mutex_lock(&proc_perms_mutex);<br>
> + do {<br>
> + line = skip_spaces(next);<br>
> + if (!*line)<br>
> + break;<br>
> +<br>
> + next = strchr(line, '\n');<br>
> + if (next)<br>
> + *next++ = '\0';<br>
> +<br>
> + if (*line != '#') {<br>
> + ret = proc_perms_line(ve, line);<br>
> + if (ret)<br>
> + break;<br>
> + }<br>
> + } while (next);<br>
> + mutex_unlock(&proc_perms_mutex);<br>
> +<br>
> + return ret ? ret : nbytes;<br>
> +}<br>
> +<br>
> +static struct cftype proc_ve_cftypes[] = {<br>
> + {<br>
> + .name = "default_proc_permissions",<br>
> + .flags = CFTYPE_ONLY_ON_ROOT,<br>
> + .seq_start = proc_perms_start,<br>
> + .seq_next = proc_perms_next,<br>
> + .seq_stop = proc_perms_stop,<br>
> + .seq_show = proc_perms_show,<br>
> + .write = proc_perms_write,<br>
> + },<br>
> + {<br>
> + .name = "proc_permissions",<br>
> + .flags = CFTYPE_NOT_ON_ROOT,<br>
> + .seq_start = proc_perms_start,<br>
> + .seq_next = proc_perms_next,<br>
> + .seq_stop = proc_perms_stop,<br>
> + .seq_show = proc_perms_show,<br>
> + .write = proc_perms_write,<br>
> + },<br>
> + { },<br>
> +};<br>
> +<br>
> +static int init_proc_ve_perms(void)<br>
> +{<br>
> + return cgroup_add_cftypes(&ve_cgrp_subsys, proc_ve_cftypes);<br>
> +}<br>
> +module_init(init_proc_ve_perms);<br>
> diff --git a/include/linux/ve.h b/include/linux/ve.h<br>
> index b037f60225bb..cba827260d07 100644<br>
> --- a/include/linux/ve.h<br>
> +++ b/include/linux/ve.h<br>
> @@ -69,6 +69,7 @@ struct ve_struct {<br>
> int fsync_enable;<br>
> <br>
> struct kmapset_key sysfs_perms_key;<br>
> + struct kmapset_key proc_perms_key;<br>
> <br>
> atomic_t netns_avail_nr;<br>
> int netns_max_nr;<br>
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c<br>
> index e58ffb22da87..d8ef28eedabd 100644<br>
> --- a/kernel/ve/ve.c<br>
> +++ b/kernel/ve/ve.c<br>
> @@ -44,6 +44,9 @@<br>
> #include "../sched/sched.h" /* For css_tg() */<br>
> <br>
> extern struct kmapset_set sysfs_ve_perms_set;<br>
> +#ifdef CONFIG_PROC_FS<br>
> +extern struct kmapset_set proc_ve_perms_set;<br>
> +#endif<br>
> <br>
> static struct kmem_cache *ve_cachep;<br>
> <br>
> @@ -771,6 +774,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_<br>
> init_rwsem(&ve->op_sem);<br>
> INIT_LIST_HEAD(&ve->ve_list);<br>
> kmapset_init_key(&ve->sysfs_perms_key);<br>
> + kmapset_init_key(&ve->proc_perms_key);<br>
> <br>
> atomic_set(&ve->arp_neigh_nr, 0);<br>
> atomic_set(&ve->nd_neigh_nr, 0);<br>
> @@ -866,6 +870,9 @@ static void ve_destroy(struct cgroup_subsys_state *css)<br>
> free_ve_devmnts(ve);<br>
> <br>
> kmapset_unlink(&ve->sysfs_perms_key, &sysfs_ve_perms_set);<br>
> +#ifdef CONFIG_PROC_FS<br>
> + kmapset_unlink(&ve->proc_perms_key, &proc_ve_perms_set);<br>
> +#endif<br>
> ve_log_destroy(ve);<br>
> ve_free_vdso(ve);<br>
> mntput(ve->devtmpfs_mnt);<br>
<br>
-- <br>
Best regards, Pavel Tikhomirov<br>
Senior Software Developer, Virtuozzo.<br>
<br>
</div>
</span></font></div>
</body>
</html>