proc: Fix parsing of mount parameters.
Subsystems (5)
PATCH 1/1 proc: Fix parsing of mount parameters.
In commit e94591d0d90c "proc: Convert proc_mount to use mount_ns" the parsing of mount parameters for the proc filesystem was broken.
The SB_KERNMOUNT for procfs happens via:
start_kernel()
rest_init()
kernel_thread()
_do_fork()
copy_process()
alloc_pid()
pid_ns_prepare_proc()
kern_mount_data()
proc_mount()
mount_ns()In mount_ns(), the kernel calls proc_fill_super() only if the superblock has not previously been set up (i.e. the first mount reference), regardless of SB_KERNMOUNT. Because the call to proc_parse_options() had been moved inside here, and the SB_KERNMOUNT uses no mount options, the option parser became a no-op.
When userspace later mounted procfs with e.g. hidepid=2, the options would be ignored.
This change backs out a part of the original cleanup and parses the procfs mount options at every mount call. Because the options currently only update the pid_ns for the mount, they are applied for all mounts of proc by that pid or childen of that pid, instantaneously. This is the same behavior as the original code.
--- fs/proc/inode.c | 4 ---- fs/proc/internal.h | 1 - fs/proc/root.c | 5 ++++- 3 files changed, 4 insertions(+), 6 deletions(-)
fs/proc/inode.c +0 -4
@@ -492,13 +492,9 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)int proc_fill_super (struct super_block *s,void *data,int silent) { -struct pid_namespace *ns =get_pid_ns (s->s_fs_info);struct inode *root_inode;int ret; -if (!proc_parse_options (data, ns)) -return -EINVAL ; -/* User space would break if executables or devices appear on proc */ s->s_iflags |=SB_I_USERNS_VISIBLE |SB_I_NOEXEC |SB_I_NODEV ; s->s_flags |=SB_NODIRATIME |SB_NOSUID |SB_NOEXEC ;
fs/proc/internal.h +0 -1
@@ -264,7 +264,6 @@ static inline void proc_tty_init(void) {}* root.c */ extern struct proc_dir_entry proc_root; -extern int proc_parse_options (char *options,struct pid_namespace *pid);extern void proc_self_init (void );extern int proc_remount (struct super_block *,int *,char *);
fs/proc/root.c +4 -1
@@ -36,7 +36,7 @@ static const match_table_t tokens = { {Opt_err, NULL}, }; -int proc_parse_options (char *options,struct pid_namespace *pid) +static int proc_parse_options (char *options,struct pid_namespace *pid) {char *p;substring_t args[MAX_OPT_ARGS ];
@@ -98,6 +98,9 @@ static struct dentry *proc_mount(struct file_system_type *fs_type, ns =task_active_pid_ns (current); } +if (!proc_parse_options (data, ns)) +return ERR_PTR (-EINVAL ); +return mount_ns (fs_type, flags, data, ns, ns->user_ns, proc_fill_super); }
Signature · 1 line
2.18.0.rc1.242.g61856ae69a-goog
Quoted 22 lines · Alistair Strachan
In commit
e94591d0d90c"proc: Convertproc_mountto usemount_ns" the parsing of mount parameters for the proc filesystem was broken.The
SB_KERNMOUNTfor procfs happens via:
start_kernel()rest_init() kernel_thread() _do_fork() copy_process() alloc_pid() pid_ns_prepare_proc() kern_mount_data() proc_mount() mount_ns()In
mount_ns(), the kernel callsproc_fill_super()only if the superblock has not previously been set up (i.e. the first mount reference), regardless ofSB_KERNMOUNT. Because the call toproc_parse_options()had been moved inside here, and theSB_KERNMOUNTuses no mount options, the option parser became a no-op.
Quoted 8 lines
When userspace later mounted procfs with e.g. hidepid=2, the options would be ignored.
This change backs out a part of the original cleanup and parses the procfs mount options at every mount call. Because the options currently only update the
pid_nsfor the mount, they are applied for all mounts of proc by that pid or childen of that pid, instantaneously. This is the same behavior as the original code.
Two years for a regression to be reported is a litte long. I think that gets out of the kneejerk immediate fix or revert phase and into thinking a little bout about what makes sense in this code.
As we say with devpts there is a very real danger of someone mounting a second instance of proc in a chroot and causing problems by either strengthening or weakening the hid pid protections for the entire pid namespace. If we go with your proposed change in behavior.
Ordinary block device filesystems (like ext4) avoid this problem by allowing a second mount and by not parsing the mount options except on remount. What proc currently does.
So I think it can be reasonably argued that the change in behavior is was an unintentional fix.
I can see an argument for failing the mount of proc if mount options are specified or if those mount options differ from the existing mount options.
proc_remount's call of proc_parse_options is definitely buggy as it can partially succeed and change the pid namespace and return an error code. That is bad error handling.
There may be an argument for making these options available in something other than a mount of proc. As they are pid namespace wide.
There may be an argument for multiple instances of proc so that it makes sense to process these options during an ordinary mount.
Ultimately what I see is that this is a difficult area of semantics that there is at least a little room for improvement on, but it is not as simple as this proposed change.
Quoted 65 lines
Fixes e94591d0d90c ("proc: Convert proc_mount to use mount_ns")Signed-off-by Alistair Strachan <[email protected]>Cc Seth Forshee <[email protected]>Cc Djalal Harouni <[email protected]>Cc "Eric W. Biederman" <[email protected]>--- fs/proc/inode.c | 4 ---- fs/proc/internal.h | 1 - fs/proc/root.c | 5 ++++- 3 files changed, 4 insertions(+), 6 deletions(-)fs/proc/inode.c +0
-4@@ -492,13 +492,9 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)int proc_fill_super (struct super_block *s,void *data,int silent) { -struct pid_namespace *ns =get_pid_ns (s->s_fs_info);struct inode *root_inode;int ret; -if (!proc_parse_options (data, ns)) -return -EINVAL ; -/* User space would break if executables or devices appear on proc */ s->s_iflags |=SB_I_USERNS_VISIBLE |SB_I_NOEXEC |SB_I_NODEV ; s->s_flags |=SB_NODIRATIME |SB_NOSUID |SB_NOEXEC ;fs/proc/internal.h +0
-1@@ -264,7 +264,6 @@ static inline void proc_tty_init(void) {}* root.c */ extern struct proc_dir_entry proc_root; -extern int proc_parse_options (char *options,struct pid_namespace *pid);extern void proc_self_init (void );extern int proc_remount (struct super_block *,int *,char *);fs/proc/root.c +4
-1@@ -36,7 +36,7 @@ static const match_table_t tokens = { {Opt_err, NULL}, }; -int proc_parse_options (char *options,struct pid_namespace *pid) +static int proc_parse_options (char *options,struct pid_namespace *pid) {char *p;substring_t args[MAX_OPT_ARGS ];@@ -98,6 +98,9 @@ static struct dentry *proc_mount(struct file_system_type *fs_type, ns =task_active_pid_ns (current); } +if (!proc_parse_options (data, ns)) +return ERR_PTR (-EINVAL ); +return mount_ns (fs_type, flags, data, ns, ns->user_ns, proc_fill_super); }
Quoted 34 lines · Eric W. Biederman
Quoted 22 lines · Alistair Strachan
In commit
e94591d0d90c"proc: Convertproc_mountto usemount_ns" the parsing of mount parameters for the proc filesystem was broken.The
SB_KERNMOUNTfor procfs happens via:
start_kernel()rest_init() kernel_thread() _do_fork() copy_process() alloc_pid() pid_ns_prepare_proc() kern_mount_data() proc_mount() mount_ns()In
mount_ns(), the kernel callsproc_fill_super()only if the superblock has not previously been set up (i.e. the first mount reference), regardless ofSB_KERNMOUNT. Because the call toproc_parse_options()had been moved inside here, and theSB_KERNMOUNTuses no mount options, the option parser became a no-op.Quoted 8 lines
When userspace later mounted procfs with e.g. hidepid=2, the options would be ignored.
This change backs out a part of the original cleanup and parses the procfs mount options at every mount call. Because the options currently only update the
pid_nsfor the mount, they are applied for all mounts of proc by that pid or childen of that pid, instantaneously. This is the same behavior as the original code.
Quoted 3 lines
Two years for a regression to be reported is a litte long. I think that gets out of the kneejerk immediate fix or revert phase and into thinking a little bout about what makes sense in this code.
Android has been using hidepid=2 for a while, but most shipping products were on 3.18 or 4.4 kernels. To us, it's a new problem.
Quoted 4 lines
As we say with devpts there is a very real danger of someone mounting a second instance of proc in a chroot and causing problems by either strengthening or weakening the hid pid protections for the entire pid namespace. If we go with your proposed change in behavior.
I guess my change does change the behavior, but it's just back to the behavior which the kernel had for a good while (~v3.3 thru v4.7).
Quoted 3 lines
Ordinary block device filesystems (like ext4) avoid this problem by allowing a second mount and by not parsing the mount options except on remount. What proc currently does.
IMHO, they're not really comparable. You'll only get kernmounts of an ext4 filesystem when finding rootfs, and in that case the user knows about the mount and can see it in /proc/mounts, so they know to use -o remount,<whatever>.
Since the first mount (where the options might have been respected) is *always* the kernmount done before init, with your change these mount options for procfs will never be respected. As userspace didn't yet mount /proc, it can't know /proc was already mounted, in order to know to use a remount to re-parse the options. The behavior was changed in a non-obvious way.
Quoted 14 lines
So I think it can be reasonably argued that the change in behavior is was an unintentional fix.
I can see an argument for failing the mount of proc if mount options are specified or if those mount options differ from the existing mount options.
proc_remount's call of
proc_parse_optionsis definitely buggy as it can partially succeed and change the pid namespace and return an error code. That is bad error handling.There may be an argument for making these options available in something other than a mount of proc. As they are pid namespace wide.
Quoted 7 lines
There may be an argument for multiple instances of proc so that it makes sense to process these options during an ordinary mount.
Ultimately what I see is that this is a difficult area of semantics that there is at least a little room for improvement on, but it is not as simple as this proposed change.
An alternative fix might be to ignore the super setup if done from a kernmount of procfs. IMO, this initial mount shouldn't be considered the first reference, because it will not pass the mount options and cannot be observed by userspace. Such a change looks complicated, though, and it would only be relevant to procfs. It might be better to roll back the cleanup and implement these semantics directly in the procfs code.
Quoted 65 lines
Quoted 65 lines
Fixes e94591d0d90c ("proc: Convert proc_mount to use mount_ns")Signed-off-by Alistair Strachan <[email protected]>Cc Seth Forshee <[email protected]>Cc Djalal Harouni <[email protected]>Cc "Eric W. Biederman" <[email protected]>--- fs/proc/inode.c | 4 ---- fs/proc/internal.h | 1 - fs/proc/root.c | 5 ++++- 3 files changed, 4 insertions(+), 6 deletions(-)fs/proc/inode.c +0
-4@@ -492,13 +492,9 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)int proc_fill_super (struct super_block *s,void *data,int silent) { -struct pid_namespace *ns =get_pid_ns (s->s_fs_info);struct inode *root_inode;int ret; -if (!proc_parse_options (data, ns)) -return -EINVAL ; -/* User space would break if executables or devices appear on proc */ s->s_iflags |=SB_I_USERNS_VISIBLE |SB_I_NOEXEC |SB_I_NODEV ; s->s_flags |=SB_NODIRATIME |SB_NOSUID |SB_NOEXEC ;fs/proc/internal.h +0
-1@@ -264,7 +264,6 @@ static inline void proc_tty_init(void) {}* root.c */ extern struct proc_dir_entry proc_root; -extern int proc_parse_options (char *options,struct pid_namespace *pid);extern void proc_self_init (void );extern int proc_remount (struct super_block *,int *,char *);fs/proc/root.c +4
-1@@ -36,7 +36,7 @@ static const match_table_t tokens = { {Opt_err, NULL}, }; -int proc_parse_options (char *options,struct pid_namespace *pid) +static int proc_parse_options (char *options,struct pid_namespace *pid) {char *p;substring_t args[MAX_OPT_ARGS ];@@ -98,6 +98,9 @@ static struct dentry *proc_mount(struct file_system_type *fs_type, ns =task_active_pid_ns (current); } +if (!proc_parse_options (data, ns)) +return ERR_PTR (-EINVAL ); +return mount_ns (fs_type, flags, data, ns, ns->user_ns, proc_fill_super); }
Quoted 35 lines · Alistair Strachan
Quoted 34 lines · Eric W. Biederman
Quoted 30 lines · Alistair Strachan
In commit
e94591d0d90c"proc: Convertproc_mountto usemount_ns" the parsing of mount parameters for the proc filesystem was broken.The
SB_KERNMOUNTfor procfs happens via:
start_kernel()rest_init() kernel_thread() _do_fork() copy_process() alloc_pid() pid_ns_prepare_proc() kern_mount_data() proc_mount() mount_ns()In
mount_ns(), the kernel callsproc_fill_super()only if the superblock has not previously been set up (i.e. the first mount reference), regardless ofSB_KERNMOUNT. Because the call toproc_parse_options()had been moved inside here, and theSB_KERNMOUNTuses no mount options, the option parser became a no-op.When userspace later mounted procfs with e.g. hidepid=2, the options would be ignored.
This change backs out a part of the original cleanup and parses the procfs mount options at every mount call. Because the options currently only update the
pid_nsfor the mount, they are applied for all mounts of proc by that pid or childen of that pid, instantaneously. This is the same behavior as the original code.
Quoted 6 lines
Quoted 3 lines
Two years for a regression to be reported is a litte long. I think that gets out of the kneejerk immediate fix or revert phase and into thinking a little bout about what makes sense in this code.
Android has been using hidepid=2 for a while, but most shipping products were on 3.18 or 4.4 kernels. To us, it's a new problem.
All that says is that no one from Android has looked at or tested a kernel (not even a stable one) since 4.4. That does not work for justifying changes/fixes to the kernel. People working together does not work to well when some of the people don't show up.
As an engineer I sympathize with your position. Whoever made the
decision that Android won't care for anything except for the best effort long term stable kernels has made your job a challenge. I do appreciate that Android at least periodically updates their kernels.
The bottom line though is that if this had been caught within the release or so of it's breaking restoring the old userspace behavior would not have been a question. Now after two years there is the question of if other people have come to depend on the new behavior.
Quoted 17 lines
Quoted 4 lines
As we say with devpts there is a very real danger of someone mounting a second instance of proc in a chroot and causing problems by either strengthening or weakening the hid pid protections for the entire pid namespace. If we go with your proposed change in behavior.
I guess my change does change the behavior, but it's just back to the behavior which the kernel had for a good while (~v3.3 thru v4.7).
Quoted 3 lines
Ordinary block device filesystems (like ext4) avoid this problem by allowing a second mount and by not parsing the mount options except on remount. What proc currently does.
IMHO, they're not really comparable. You'll only get kernmounts of an ext4 filesystem when finding rootfs, and in that case the user knows about the mount and can see it in
/proc/mounts, so they know to use -o remount,<whatever>.
Quoted 6 lines
Since the first mount (where the options might have been respected) is *always* the kernmount done before init, with your change these mount options for procfs will never be respected. As userspace didn't yet mount /proc, it can't know /proc was already mounted, in order to know to use a remount to re-parse the options. The behavior was changed in a non-obvious way.
Please not it is fundamentally required if the in kernel superblocks are going to be shared.
Quoted 22 lines
Quoted 14 lines
So I think it can be reasonably argued that the change in behavior is was an unintentional fix.
I can see an argument for failing the mount of proc if mount options are specified or if those mount options differ from the existing mount options.
proc_remount's call of
proc_parse_optionsis definitely buggy as it can partially succeed and change the pid namespace and return an error code. That is bad error handling.There may be an argument for making these options available in something other than a mount of proc. As they are pid namespace wide.
Quoted 7 lines
There may be an argument for multiple instances of proc so that it makes sense to process these options during an ordinary mount.
Ultimately what I see is that this is a difficult area of semantics that there is at least a little room for improvement on, but it is not as simple as this proposed change.
Quoted 7 lines
An alternative fix might be to ignore the super setup if done from a kernmount of procfs. IMO, this initial mount shouldn't be considered the first reference, because it will not pass the mount options and cannot be observed by userspace. Such a change looks complicated, though, and it would only be relevant to procfs. It might be better to roll back the cleanup and implement these semantics directly in the procfs code.
That would be straight forward if we could get rid of proc_mnt. Then there would not need to be an internal kernel mount. Which would get us there and that would probably be a nice cleanup.
We can hand wave away the uml mconsole and sysctl syscall uses of proc_mnt. The tricky case is proc_flush_task. That requires someone having a reference to the proc super block and it noticably keeps the amount of memory consumed by the proc filesystem down. Enough so that people notice when it is not performing well.
I have heard of people running Android in a chroot on chrome books, and I have heard of people running normal linux chroots on Android phones. So chroots and people mounting proc for a second time are definitely things to worry about in the real world. So I strongly suspect the old behavior is quite risky in those real world situations.
At this moment I really hate mount options to proc. They are quite difficult to make work well and in a non-surprising way.
Eric
PATCH 1/1 proc: Simplify and fix proc by removing the kernel mount
Today there are three users of proc_mnt.
- The legacy sysctl system call implementation.
- The uml mconsole driver.
- The process cleanup function proc_flush_task.
The first two are slow path and for the sysctl code there is even a case
for removing it. A new wrapper file_open_proc to mount and unmount
proc around file_open_root, to keep the first two cases working.
For proc_flush_task optimize it so that if all of the references to the pid are gone it doesn't examine proc, and then have proc_flush_task take a reference to the appropriate already mounted proc instances. Mounting proc here unconditionaly would not make sense as the goal is simply to flush dentries from an already mounted proc instance.
The two extra atomics operations in exit are not my favorite but given that exit is already almost completely serialized with the task lock I don't think this change will be noticable performance wise.
This removes a lot of non-trivial complexity in the mounting and unmounting of proc making the code easier to maintain.
This also means userspace will trigger proc_fill_super and in doing so proc will process the mount options passed to mount by userspace. This fixes an accidental regression where proc was not processing mount options and broke the Android's usage of the proc hidepid option.
--- arch/um/drivers/mconsole_kern.c | 4 ++-- fs/proc/base.c | 33 +++++++++++++++++++++++++-------- fs/proc/inode.c | 7 ++++++- fs/proc/root.c | 29 ++++++++++++----------------- include/linux/pid_namespace.h | 3 +-- include/linux/proc_ns.h | 7 ++----- kernel/pid.c | 8 -------- kernel/pid_namespace.c | 7 ------- kernel/sysctl_binary.c | 5 ++--- 9 files changed, 50 insertions(+), 53 deletions(-)
arch/um/drivers/mconsole_kern.c +2 -2
@@ -27,6 +27,7 @@#include <linux/file.h> #include <linux/uaccess.h> #include <asm/switch_to.h> +#include <linux/proc_ns.h> #include <init.h> #include <irq_kern.h>
@@ -124,7 +125,6 @@ void mconsole_log(struct mc_request *req)void mconsole_proc (struct mc_request *req) { -struct vfsmount *mnt =task_active_pid_ns (current)->proc_mnt;char *buf;int len;struct file *file;
@@ -135,7 +135,7 @@ void mconsole_proc(struct mc_request *req) ptr +=strlen ("proc" ); ptr =skip_spaces (ptr); - file =file_open_root (mnt->mnt_root, mnt, ptr,O_RDONLY ,0 ); + file =file_open_proc (ptr,O_RDONLY ,0 );if (IS_ERR (file)) {mconsole_reply (req,"Failed to open file" ,1 ,0 );printk (KERN_ERR "open /proc/%s: %ld\n" , ptr,PTR_ERR (file));
fs/proc/base.c +25 -8
@@ -3052,7 +3052,7 @@ static const struct inode_operations proc_tgid_base_inode_operations = { .permission = proc_pid_permission, }; -static void proc_flush_task_mnt (struct vfsmount *mnt,pid_t pid,pid_t tgid) +static void proc_flush_task_root (struct dentry *proc_root,pid_t pid,pid_t tgid) {struct dentry *dentry, *leader, *dir;char buf[10 +1 ];
@@ -3061,7 +3061,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid) name.name = buf; name.len =snprintf (buf,sizeof (buf),"%u" , pid);/* no ->d_hash() rejects on procfs */ - dentry =d_hash_and_lookup (mnt->mnt_root, &name); + dentry =d_hash_and_lookup (proc_root, &name);if (dentry) {d_invalidate (dentry);dput (dentry);
@@ -3072,7 +3072,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid) name.name = buf; name.len =snprintf (buf,sizeof (buf),"%u" , tgid); - leader =d_hash_and_lookup (mnt->mnt_root, &name); + leader =d_hash_and_lookup (proc_root, &name);if (!leader) goto out;
@@ -3102,8 +3102,8 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)* @task: task that should be flushed. * * When flushing dentries from proc, one needs to flush them from global -* proc (proc_mnt) and from all the namespaces' procs this task was seen -* in. This call is supposed to do all of this job. +* proc and from all the namespaces' procs this task was seen in. This call +* is supposed to do all of this job. * * Looks in the dcache for * /proc/@pid
@@ -3126,16 +3126,33 @@ void proc_flush_task(struct task_struct *task) {int i;struct pid *pid, *tgid; -struct upid *upid; +struct upid *upid, *utgid; pid =task_pid (task); tgid =task_tgid (task); +/* Proc directories and files takes a reference to pid */ +if (atomic_read (&pid->count) == (pid == tgid) ?2 :1 ) +return ; + +rcu_read_lock ();for (i =0 ; i <= pid->level; i++) { +struct super_block *sb; upid = &pid->numbers[i]; -proc_flush_task_mnt (upid->ns->proc_mnt, upid->nr, - tgid->numbers[i].nr); + utgid = &tgid->numbers[i]; + +smp_rmb ();/* Paired with proc_kill_sb */ + sb =READ_ONCE (upid->ns->proc_super); +if (!sb || !atomic_inc_not_zero (&sb->s_active)) +continue ; +rcu_read_unlock (); + +proc_flush_task_root (sb->s_root, upid->nr, utgid->nr); +deactivate_super (sb); + +rcu_read_lock (); } +rcu_read_unlock (); } static int proc_pid_instantiate(struct inode *dir,
fs/proc/inode.c +6 -1
@@ -532,5 +532,10 @@ int proc_fill_super(struct super_block *s, void *data, int silent)if (ret) {return ret; } -return proc_setup_thread_self (s); + ret =proc_setup_thread_self (s); + +WRITE_ONCE (ns->proc_super, s); +smp_wmb ();/* Let proc_flush_task know sb->s_root is valid */ + +return ret; }
fs/proc/root.c +12 -17
@@ -89,14 +89,7 @@ int proc_remount(struct super_block *sb, int *flags, char *data)static struct dentry *proc_mount (struct file_system_type *fs_type,int flags,const char *dev_name,void *data) { -struct pid_namespace *ns; - -if (flags &SB_KERNMOUNT ) { - ns = data; - data =NULL ; - }else { - ns =task_active_pid_ns (current); - } +struct pid_namespace *ns =task_active_pid_ns (current);return mount_ns (fs_type, flags, data, ns, ns->user_ns, proc_fill_super); }
@@ -110,6 +103,8 @@ static void proc_kill_sb(struct super_block *sb)dput (ns->proc_self);if (ns->proc_thread_self)dput (ns->proc_thread_self); +WRITE_ONCE (ns->proc_super,NULL ); +smp_wmb ();/* Let proc_flush_task know the sb is gone */ kill_anon_super (sb);put_pid_ns (ns); }
@@ -208,19 +203,19 @@ struct proc_dir_entry proc_root = { .inline_name = "/proc", }; -int pid_ns_prepare_proc (struct pid_namespace *ns) +#if defined(CONFIG_SYSCTL_SYSCALL ) || defined(CONFIG_MCONSOLE ) +struct file *file_open_proc (const char *pathname,int flags,umode_t mode) {struct vfsmount *mnt; +struct file *file; - mnt =kern_mount_data (&proc_fs_type, ns); + mnt =vfs_kern_mount (&proc_fs_type,0 , proc_fs_type.name,NULL );if (IS_ERR (mnt)) -return PTR_ERR (mnt); +return ERR_CAST (mnt); - ns->proc_mnt = mnt; -return 0 ; -} + file =file_open_root (mnt->mnt_root, mnt, pathname, flags, mode); +mntput (mnt); -void pid_ns_release_proc (struct pid_namespace *ns) -{ -kern_unmount (ns->proc_mnt); +return file; } +#endif
include/linux/pid_namespace.h +1 -2
@@ -31,7 +31,7 @@ struct pid_namespace {unsigned int level;struct pid_namespace *parent;#ifdef CONFIG_PROC_FS -struct vfsmount *proc_mnt; +struct super_block *proc_super;struct dentry *proc_self;struct dentry *proc_thread_self;#endif
@@ -40,7 +40,6 @@ struct pid_namespace {#endif struct user_namespace *user_ns;struct ucounts *ucounts; -struct work_struct proc_work;kgid_t pid_gid;int hide_pid;int reboot;/* group exit code if this pidns was rebooted */
include/linux/proc_ns.h +2 -5
@@ -47,16 +47,11 @@ enum {#ifdef CONFIG_PROC_FS -extern int pid_ns_prepare_proc (struct pid_namespace *ns); -extern void pid_ns_release_proc (struct pid_namespace *ns);extern int proc_alloc_inum (unsigned int *pino);extern void proc_free_inum (unsigned int inum);#else /* CONFIG_PROC_FS */ -static inline int pid_ns_prepare_proc (struct pid_namespace *ns) {return 0 ; } -static inline void pid_ns_release_proc (struct pid_namespace *ns) {} -static inline int proc_alloc_inum (unsigned int *inum) { *inum =1 ;
@@ -86,4 +81,6 @@ extern int ns_get_name(char *buf, size_t size, struct task_struct *task, const struct proc_ns_operations *ns_ops);extern void nsfs_init (void ); +extern struct file *file_open_proc (const char *pathname,int flags,umode_t mode); +#endif /* _LINUX_PROC_NS_H */
kernel/pid.c +0 -8
@@ -143,9 +143,6 @@ void free_pid(struct pid *pid)/* Handle a fork failure of the first process */ WARN_ON (ns->child_reaper); ns->pid_allocated =0 ; -/* fall through */ -case 0 : -schedule_work (&ns->proc_work);break ; }
@@ -204,11 +201,6 @@ struct pid *alloc_pid(struct pid_namespace *ns) tmp = tmp->parent; } -if (unlikely (is_child_reaper (pid))) { -if (pid_ns_prepare_proc (ns)) - goto out_free; - } -get_pid_ns (ns);atomic_set (&pid->count,1 );for (type =0 ; type <PIDTYPE_MAX ; ++type)
kernel/pid_namespace.c +0 -7
@@ -58,12 +58,6 @@ static struct kmem_cache *create_pid_cachep(unsigned int level)return READ_ONCE (*pkc); } -static void proc_cleanup_work (struct work_struct *work) -{ - struct pid_namespace *ns = container_of(work, struct pid_namespace, proc_work); -pid_ns_release_proc (ns); -} -static struct ucounts *inc_pid_namespaces (struct user_namespace *ns) {return inc_ucount (ns,current_euid (),UCOUNT_PID_NAMESPACES );
@@ -115,7 +109,6 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns ns->user_ns =get_user_ns (user_ns); ns->ucounts = ucounts; ns->pid_allocated =PIDNS_ADDING ; -INIT_WORK (&ns->proc_work, proc_cleanup_work);return ns;
kernel/sysctl_binary.c +2 -3
@@ -17,6 +17,7 @@#include <linux/uuid.h> #include <linux/slab.h> #include <linux/compat.h> +#include <linux/proc_ns.h> #ifdef CONFIG_SYSCTL_SYSCALL
@@ -1278,7 +1279,6 @@ static ssize_t binary_sysctl(const int *name, int nlen, void __user *oldval, size_t oldlen, void __user *newval, size_t newlen) { const struct bin_table *table = NULL; -struct vfsmount *mnt;struct file *file;ssize_t result;char *pathname;
@@ -1301,8 +1301,7 @@ static ssize_t binary_sysctl(const int *name, int nlen, goto out_putname; } - mnt =task_active_pid_ns (current)->proc_mnt; - file =file_open_root (mnt->mnt_root, mnt, pathname, flags,0 ); + file =file_open_proc (pathname, flags,0 ); result =PTR_ERR (file);if (IS_ERR (file)) goto out_putname;
Signature · 1 line
2.17.1
PATCH v2 1/1 proc: Simplify and fix proc by removing the kernel mount
Today there are three users of proc_mnt.
- The legacy sysctl system call implementation.
- The uml mconsole driver.
- The process cleanup function proc_flush_task.
The first two are slow path and essentially unused. I expect soon we will be able to remove the legacy sysctl system call entirely. To keep them working for now a new wrapper file_open_proc_is added to mount and unmount proc around file_open_root. Which nicely removes the need for a always mounted proc instance for these cases.
Handling proc_flush_task which is regularly used requires a little more work. First I optimize proc_flush_task to do nothing where there is evidence that there are no entries in proc, by looking at pid->count. Then I carefully update proc_fill_super and proc_kill_sb to maintain a ns->proc_super pointer to the super block for proc. This allows proc_flush_task to find the appropriate instance of proc via rcu.
Once the appropriate instance of proc is found in proc_flush_task atomic_inc_not_zero is used to increase the s_active count ensuring proc_kill_sb will not be called, until the superblock is deactivated. This makes it safe to inspect the instance of proc and invalidate any dentries that mention the exiting task.
The two extra atomics operations in exit are not my favorite but given that exit is already almost completely serialized with the task lock I do not expect this change will be measurable.
The benefit for all of this change is that one of the most error prone and tricky parts of the pid namespace implementation, maintaining kernel mounts of proc is removed.
In addition removing the unnecessary complexity of the kernel mount fixes a regression that caused the proc mount options to be ignored. Now that the initial mount of proc comes from userspace, those mount options are again honored. This fixes Android's usage of the proc hidepid option.
---
Alistair if you can test and confirm this fixes your issue I will add your tested by and send the fix to Linus.
Since my earlier posting I have spot tested this. Fixed a few bugs that showed up and verified my changes work. So I think this is ready to go unless someone looks at this and in testing or code review spots a bug.
Eric
arch/um/drivers/mconsole_kern.c | 4 ++-- fs/proc/base.c | 36 ++++++++++++++++++++++++++------- fs/proc/inode.c | 5 ++++- fs/proc/root.c | 28 ++++++++++--------------- include/linux/pid_namespace.h | 3 +-- include/linux/proc_ns.h | 7 ++----- kernel/pid.c | 8 -------- kernel/pid_namespace.c | 7 ------- kernel/sysctl_binary.c | 5 ++--- 9 files changed, 51 insertions(+), 52 deletions(-)
arch/um/drivers/mconsole_kern.c +2 -2
@@ -27,6 +27,7 @@#include <linux/file.h> #include <linux/uaccess.h> #include <asm/switch_to.h> +#include <linux/proc_ns.h> #include <init.h> #include <irq_kern.h>
@@ -124,7 +125,6 @@ void mconsole_log(struct mc_request *req)void mconsole_proc (struct mc_request *req) { -struct vfsmount *mnt =task_active_pid_ns (current)->proc_mnt;char *buf;int len;struct file *file;
@@ -135,7 +135,7 @@ void mconsole_proc(struct mc_request *req) ptr +=strlen ("proc" ); ptr =skip_spaces (ptr); - file =file_open_root (mnt->mnt_root, mnt, ptr,O_RDONLY ,0 ); + file =file_open_proc (ptr,O_RDONLY ,0 );if (IS_ERR (file)) {mconsole_reply (req,"Failed to open file" ,1 ,0 );printk (KERN_ERR "open /proc/%s: %ld\n" , ptr,PTR_ERR (file));
fs/proc/base.c +29 -7
@@ -3052,7 +3052,7 @@ static const struct inode_operations proc_tgid_base_inode_operations = { .permission = proc_pid_permission, }; -static void proc_flush_task_mnt (struct vfsmount *mnt,pid_t pid,pid_t tgid) +static void proc_flush_task_root (struct dentry *proc_root,pid_t pid,pid_t tgid) {struct dentry *dentry, *leader, *dir;char buf[10 +1 ];
@@ -3061,7 +3061,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid) name.name = buf; name.len =snprintf (buf,sizeof (buf),"%u" , pid);/* no ->d_hash() rejects on procfs */ - dentry =d_hash_and_lookup (mnt->mnt_root, &name); + dentry =d_hash_and_lookup (proc_root, &name);if (dentry) {d_invalidate (dentry);dput (dentry);
@@ -3072,7 +3072,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid) name.name = buf; name.len =snprintf (buf,sizeof (buf),"%u" , tgid); - leader =d_hash_and_lookup (mnt->mnt_root, &name); + leader =d_hash_and_lookup (proc_root, &name);if (!leader) goto out;
@@ -3102,8 +3102,8 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)* @task: task that should be flushed. * * When flushing dentries from proc, one needs to flush them from global -* proc (proc_mnt) and from all the namespaces' procs this task was seen -* in. This call is supposed to do all of this job. +* proc and from all the namespaces' procs this task was seen in. This call +* is supposed to do all of this job. * * Looks in the dcache for * /proc/@pid
@@ -3127,15 +3127,37 @@ void proc_flush_task(struct task_struct *task)int i;struct pid *pid, *tgid;struct upid *upid; +int expected =1 ; pid =task_pid (task); tgid =task_tgid (task); +if (thread_group_leader (task)) { +if (task_pgrp (task) == pid) + expected++; +if (task_session (task) == pid) + expected++; + } + +/* Nothing to do if proc inodes have not take a reference to pid */ +if (atomic_read (&pid->count) == expected) +return ; +rcu_read_lock ();for (i =0 ; i <= pid->level; i++) { +struct super_block *sb; upid = &pid->numbers[i]; -proc_flush_task_mnt (upid->ns->proc_mnt, upid->nr, - tgid->numbers[i].nr); + + sb =rcu_dereference (upid->ns->proc_super); +if (!sb || !atomic_inc_not_zero (&sb->s_active)) +continue ; +rcu_read_unlock (); + +proc_flush_task_root (sb->s_root, upid->nr, tgid->numbers[i].nr); +deactivate_super (sb); + +rcu_read_lock (); } +rcu_read_unlock (); } static int proc_pid_instantiate(struct inode *dir,
fs/proc/inode.c +4 -1
@@ -532,5 +532,8 @@ int proc_fill_super(struct super_block *s, void *data, int silent)if (ret) {return ret; } -return proc_setup_thread_self (s); + ret =proc_setup_thread_self (s); + +rcu_assign_pointer (ns->proc_super, s); +return ret; }
fs/proc/root.c +11 -17
@@ -89,14 +89,7 @@ int proc_remount(struct super_block *sb, int *flags, char *data)static struct dentry *proc_mount (struct file_system_type *fs_type,int flags,const char *dev_name,void *data) { -struct pid_namespace *ns; - -if (flags &SB_KERNMOUNT ) { - ns = data; - data =NULL ; - }else { - ns =task_active_pid_ns (current); - } +struct pid_namespace *ns =task_active_pid_ns (current);return mount_ns (fs_type, flags, data, ns, ns->user_ns, proc_fill_super); }
@@ -106,6 +99,7 @@ static void proc_kill_sb(struct super_block *sb)struct pid_namespace *ns; ns = (struct pid_namespace *)sb->s_fs_info; +rcu_assign_pointer (ns->proc_super,NULL );if (ns->proc_self)dput (ns->proc_self);if (ns->proc_thread_self)
@@ -208,19 +202,19 @@ struct proc_dir_entry proc_root = { .inline_name = "/proc", }; -int pid_ns_prepare_proc (struct pid_namespace *ns) +#if defined(CONFIG_SYSCTL_SYSCALL ) || defined(CONFIG_MCONSOLE ) +struct file *file_open_proc (const char *pathname,int flags,umode_t mode) {struct vfsmount *mnt; +struct file *file; - mnt =kern_mount_data (&proc_fs_type, ns); + mnt =kern_mount (&proc_fs_type);if (IS_ERR (mnt)) -return PTR_ERR (mnt); +return ERR_CAST (mnt); - ns->proc_mnt = mnt; -return 0 ; -} + file =file_open_root (mnt->mnt_root, mnt, pathname, flags, mode); +kern_unmount (mnt); -void pid_ns_release_proc (struct pid_namespace *ns) -{ -kern_unmount (ns->proc_mnt); +return file; } +#endif
include/linux/pid_namespace.h +1 -2
@@ -31,7 +31,7 @@ struct pid_namespace {unsigned int level;struct pid_namespace *parent;#ifdef CONFIG_PROC_FS -struct vfsmount *proc_mnt; +struct super_block __rcu *proc_super;struct dentry *proc_self;struct dentry *proc_thread_self;#endif
@@ -40,7 +40,6 @@ struct pid_namespace {#endif struct user_namespace *user_ns;struct ucounts *ucounts; -struct work_struct proc_work;kgid_t pid_gid;int hide_pid;int reboot;/* group exit code if this pidns was rebooted */
include/linux/proc_ns.h +2 -5
@@ -47,16 +47,11 @@ enum {#ifdef CONFIG_PROC_FS -extern int pid_ns_prepare_proc (struct pid_namespace *ns); -extern void pid_ns_release_proc (struct pid_namespace *ns);extern int proc_alloc_inum (unsigned int *pino);extern void proc_free_inum (unsigned int inum);#else /* CONFIG_PROC_FS */ -static inline int pid_ns_prepare_proc (struct pid_namespace *ns) {return 0 ; } -static inline void pid_ns_release_proc (struct pid_namespace *ns) {} -static inline int proc_alloc_inum (unsigned int *inum) { *inum =1 ;
@@ -86,4 +81,6 @@ extern int ns_get_name(char *buf, size_t size, struct task_struct *task, const struct proc_ns_operations *ns_ops);extern void nsfs_init (void ); +extern struct file *file_open_proc (const char *pathname,int flags,umode_t mode); +#endif /* _LINUX_PROC_NS_H */
kernel/pid.c +0 -8
@@ -143,9 +143,6 @@ void free_pid(struct pid *pid)/* Handle a fork failure of the first process */ WARN_ON (ns->child_reaper); ns->pid_allocated =0 ; -/* fall through */ -case 0 : -schedule_work (&ns->proc_work);break ; }
@@ -204,11 +201,6 @@ struct pid *alloc_pid(struct pid_namespace *ns) tmp = tmp->parent; } -if (unlikely (is_child_reaper (pid))) { -if (pid_ns_prepare_proc (ns)) - goto out_free; - } -get_pid_ns (ns);atomic_set (&pid->count,1 );for (type =0 ; type <PIDTYPE_MAX ; ++type)
kernel/pid_namespace.c +0 -7
@@ -58,12 +58,6 @@ static struct kmem_cache *create_pid_cachep(unsigned int level)return READ_ONCE (*pkc); } -static void proc_cleanup_work (struct work_struct *work) -{ - struct pid_namespace *ns = container_of(work, struct pid_namespace, proc_work); -pid_ns_release_proc (ns); -} -static struct ucounts *inc_pid_namespaces (struct user_namespace *ns) {return inc_ucount (ns,current_euid (),UCOUNT_PID_NAMESPACES );
@@ -115,7 +109,6 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns ns->user_ns =get_user_ns (user_ns); ns->ucounts = ucounts; ns->pid_allocated =PIDNS_ADDING ; -INIT_WORK (&ns->proc_work, proc_cleanup_work);return ns;
kernel/sysctl_binary.c +2 -3
@@ -17,6 +17,7 @@#include <linux/uuid.h> #include <linux/slab.h> #include <linux/compat.h> +#include <linux/proc_ns.h> #ifdef CONFIG_SYSCTL_SYSCALL
@@ -1278,7 +1279,6 @@ static ssize_t binary_sysctl(const int *name, int nlen, void __user *oldval, size_t oldlen, void __user *newval, size_t newlen) { const struct bin_table *table = NULL; -struct vfsmount *mnt;struct file *file;ssize_t result;char *pathname;
@@ -1301,8 +1301,7 @@ static ssize_t binary_sysctl(const int *name, int nlen, goto out_putname; } - mnt =task_active_pid_ns (current)->proc_mnt; - file =file_open_root (mnt->mnt_root, mnt, pathname, flags,0 ); + file =file_open_proc (pathname, flags,0 ); result =PTR_ERR (file);if (IS_ERR (file)) goto out_putname;
Signature · 1 line
2.17.1
Re: [PATCH v2] proc: Simplify and fix proc by removing the kernel mount
Hi Eric,
Thanks a lot for looking into this problem.
Quoted 40 lines · Eric W. Biederman
Today there are three users of
proc_mnt.
- The legacy sysctl system call implementation.
- The uml mconsole driver.
- The process cleanup functionproc_flush_task.The first two are slow path and essentially unused. I expect soon we will be able to remove the legacy sysctl system call entirely. To keep them working for now a new wrapper
file_open_proc_isadded to mount and unmount proc aroundfile_open_root. Which nicely removes the need for a always mounted proc instance for these cases.Handling
proc_flush_taskwhich is regularly used requires a little more work. First I optimizeproc_flush_taskto do nothing where there is evidence that there are no entries in proc, by looking atpid->count. Then I carefully updateproc_fill_superandproc_kill_sbto maintain ans->proc_superpointer to the super block for proc. This allowsproc_flush_taskto find the appropriate instance of proc via rcu.Once the appropriate instance of proc is found in
proc_flush_taskatomic_inc_not_zerois used to increase thes_activecount ensuringproc_kill_sbwill not be called, until the superblock is deactivated. This makes it safe to inspect the instance of proc and invalidate any dentries that mention the exiting task.The two extra atomics operations in exit are not my favorite but given that exit is already almost completely serialized with the task lock I do not expect this change will be measurable.
The benefit for all of this change is that one of the most error prone and tricky parts of the pid namespace implementation, maintaining kernel mounts of proc is removed.
In addition removing the unnecessary complexity of the kernel mount fixes a regression that caused the proc mount options to be ignored. Now that the initial mount of proc comes from userspace, those mount options are again honored. This fixes Android's usage of the proc hidepid option.
Quoted 8 lines
Reported-by Alistair Strachan <[email protected]>Fixes e94591d0d90c ("proc: Convert proc_mount to use mount_ns.")Signed-off-by "Eric W. Biederman" <[email protected]>---
Alistair if you can test and confirm this fixes your issue I will add your tested by and send the fix to Linus.
I tested v2 with both UML and qemu-system-x86_64 / ARCH=x86_64 against 4.18-rc1, 4.14 and 4.9 and I couldn't break it. The hidepid problem is resolved, and the mount flags can now only be specified on the first userspace mount for that pid namespace.
Quoted 3 lines
Since my earlier posting I have spot tested this. Fixed a few bugs that showed up and verified my changes work. So I think this is ready to go unless someone looks at this and in testing or code review spots a bug.
Agreed!
Quoted 327 lines
Eric
arch/um/drivers/mconsole_kern.c | 4 ++-- fs/proc/base.c | 36 ++++++++++++++++++++++++++------- fs/proc/inode.c | 5 ++++- fs/proc/root.c | 28 ++++++++++--------------- include/linux/pid_namespace.h | 3 +-- include/linux/proc_ns.h | 7 ++----- kernel/pid.c | 8 -------- kernel/pid_namespace.c | 7 ------- kernel/sysctl_binary.c | 5 ++--- 9 files changed, 51 insertions(+), 52 deletions(-)arch/um/drivers/mconsole_kern.c +2
-2@@ -27,6 +27,7 @@#include <linux/file.h> #include <linux/uaccess.h> #include <asm/switch_to.h> +#include <linux/proc_ns.h> #include <init.h> #include <irq_kern.h> @@ -124,7 +125,6 @@ void mconsole_log(struct mc_request *req)void mconsole_proc (struct mc_request *req) { -struct vfsmount *mnt =task_active_pid_ns (current)->proc_mnt;char *buf;int len;struct file *file;@@ -135,7 +135,7 @@ void mconsole_proc(struct mc_request *req) ptr +=strlen ("proc" ); ptr =skip_spaces (ptr); - file =file_open_root (mnt->mnt_root, mnt, ptr,O_RDONLY ,0 ); + file =file_open_proc (ptr,O_RDONLY ,0 );if (IS_ERR (file)) {mconsole_reply (req,"Failed to open file" ,1 ,0 );printk (KERN_ERR "open /proc/%s: %ld\n" , ptr,PTR_ERR (file));fs/proc/base.c +29
-7@@ -3052,7 +3052,7 @@ static const struct inode_operations proc_tgid_base_inode_operations = { .permission = proc_pid_permission, }; -static void proc_flush_task_mnt (struct vfsmount *mnt,pid_t pid,pid_t tgid) +static void proc_flush_task_root (struct dentry *proc_root,pid_t pid,pid_t tgid) {struct dentry *dentry, *leader, *dir;char buf[10 +1 ];@@ -3061,7 +3061,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid) name.name = buf; name.len =snprintf (buf,sizeof (buf),"%u" , pid);/* no ->d_hash() rejects on procfs */ - dentry =d_hash_and_lookup (mnt->mnt_root, &name); + dentry =d_hash_and_lookup (proc_root, &name);if (dentry) {d_invalidate (dentry);dput (dentry);@@ -3072,7 +3072,7 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid) name.name = buf; name.len =snprintf (buf,sizeof (buf),"%u" , tgid); - leader =d_hash_and_lookup (mnt->mnt_root, &name); + leader =d_hash_and_lookup (proc_root, &name);if (!leader) goto out;@@ -3102,8 +3102,8 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)* @task: task that should be flushed. * * When flushing dentries from proc, one needs to flush them from global -* proc (proc_mnt) and from all the namespaces' procs this task was seen -* in. This call is supposed to do all of this job. +* proc and from all the namespaces' procs this task was seen in. This call +* is supposed to do all of this job. * * Looks in the dcache for * /proc/@pid @@ -3127,15 +3127,37 @@ void proc_flush_task(struct task_struct *task)int i;struct pid *pid, *tgid;struct upid *upid; +int expected =1 ; pid =task_pid (task); tgid =task_tgid (task); +if (thread_group_leader (task)) { +if (task_pgrp (task) == pid) + expected++; +if (task_session (task) == pid) + expected++; + } + +/* Nothing to do if proc inodes have not take a reference to pid */ +if (atomic_read (&pid->count) == expected) +return ; +rcu_read_lock ();for (i =0 ; i <= pid->level; i++) { +struct super_block *sb; upid = &pid->numbers[i]; -proc_flush_task_mnt (upid->ns->proc_mnt, upid->nr, - tgid->numbers[i].nr); + + sb =rcu_dereference (upid->ns->proc_super); +if (!sb || !atomic_inc_not_zero (&sb->s_active)) +continue ; +rcu_read_unlock (); + +proc_flush_task_root (sb->s_root, upid->nr, tgid->numbers[i].nr); +deactivate_super (sb); + +rcu_read_lock (); } +rcu_read_unlock (); } static int proc_pid_instantiate(struct inode *dir,fs/proc/inode.c +4
-1@@ -532,5 +532,8 @@ int proc_fill_super(struct super_block *s, void *data, int silent)if (ret) {return ret; } -return proc_setup_thread_self (s); + ret =proc_setup_thread_self (s); + +rcu_assign_pointer (ns->proc_super, s); +return ret; }fs/proc/root.c +11
-17@@ -89,14 +89,7 @@ int proc_remount(struct super_block *sb, int *flags, char *data)static struct dentry *proc_mount (struct file_system_type *fs_type,int flags,const char *dev_name,void *data) { -struct pid_namespace *ns; - -if (flags &SB_KERNMOUNT ) { - ns = data; - data =NULL ; - }else { - ns =task_active_pid_ns (current); - } +struct pid_namespace *ns =task_active_pid_ns (current);return mount_ns (fs_type, flags, data, ns, ns->user_ns, proc_fill_super); }@@ -106,6 +99,7 @@ static void proc_kill_sb(struct super_block *sb)struct pid_namespace *ns; ns = (struct pid_namespace *)sb->s_fs_info; +rcu_assign_pointer (ns->proc_super,NULL );if (ns->proc_self)dput (ns->proc_self);if (ns->proc_thread_self)@@ -208,19 +202,19 @@ struct proc_dir_entry proc_root = { .inline_name = "/proc", }; -int pid_ns_prepare_proc (struct pid_namespace *ns) +#if defined(CONFIG_SYSCTL_SYSCALL ) || defined(CONFIG_MCONSOLE ) +struct file *file_open_proc (const char *pathname,int flags,umode_t mode) {struct vfsmount *mnt; +struct file *file; - mnt =kern_mount_data (&proc_fs_type, ns); + mnt =kern_mount (&proc_fs_type);if (IS_ERR (mnt)) -return PTR_ERR (mnt); +return ERR_CAST (mnt); - ns->proc_mnt = mnt; -return 0 ; -} + file =file_open_root (mnt->mnt_root, mnt, pathname, flags, mode); +kern_unmount (mnt); -void pid_ns_release_proc (struct pid_namespace *ns) -{ -kern_unmount (ns->proc_mnt); +return file; } +#endif include/linux/pid_namespace.h +1
-2@@ -31,7 +31,7 @@ struct pid_namespace {unsigned int level;struct pid_namespace *parent;#ifdef CONFIG_PROC_FS -struct vfsmount *proc_mnt; +struct super_block __rcu *proc_super;struct dentry *proc_self;struct dentry *proc_thread_self;#endif @@ -40,7 +40,6 @@ struct pid_namespace {#endif struct user_namespace *user_ns;struct ucounts *ucounts; -struct work_struct proc_work;kgid_t pid_gid;int hide_pid;int reboot;/* group exit code if this pidns was rebooted */ include/linux/proc_ns.h +2
-5@@ -47,16 +47,11 @@ enum {#ifdef CONFIG_PROC_FS -extern int pid_ns_prepare_proc (struct pid_namespace *ns); -extern void pid_ns_release_proc (struct pid_namespace *ns);extern int proc_alloc_inum (unsigned int *pino);extern void proc_free_inum (unsigned int inum);#else /* CONFIG_PROC_FS */ -static inline int pid_ns_prepare_proc (struct pid_namespace *ns) {return 0 ; } -static inline void pid_ns_release_proc (struct pid_namespace *ns) {} -static inline int proc_alloc_inum (unsigned int *inum) { *inum =1 ;@@ -86,4 +81,6 @@ extern int ns_get_name(char *buf, size_t size, struct task_struct *task, const struct proc_ns_operations *ns_ops);extern void nsfs_init (void ); +extern struct file *file_open_proc (const char *pathname,int flags,umode_t mode); +#endif /* _LINUX_PROC_NS_H */ kernel/pid.c +0
-8@@ -143,9 +143,6 @@ void free_pid(struct pid *pid)/* Handle a fork failure of the first process */ WARN_ON (ns->child_reaper); ns->pid_allocated =0 ; -/* fall through */ -case 0 : -schedule_work (&ns->proc_work);break ; }@@ -204,11 +201,6 @@ struct pid *alloc_pid(struct pid_namespace *ns) tmp = tmp->parent; } -if (unlikely (is_child_reaper (pid))) { -if (pid_ns_prepare_proc (ns)) - goto out_free; - } -get_pid_ns (ns);atomic_set (&pid->count,1 );for (type =0 ; type <PIDTYPE_MAX ; ++type)kernel/pid_namespace.c +0
-7@@ -58,12 +58,6 @@ static struct kmem_cache *create_pid_cachep(unsigned int level)return READ_ONCE (*pkc); } -static void proc_cleanup_work (struct work_struct *work) -{ - struct pid_namespace *ns = container_of(work, struct pid_namespace, proc_work); -pid_ns_release_proc (ns); -} -static struct ucounts *inc_pid_namespaces (struct user_namespace *ns) {return inc_ucount (ns,current_euid (),UCOUNT_PID_NAMESPACES );@@ -115,7 +109,6 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns ns->user_ns =get_user_ns (user_ns); ns->ucounts = ucounts; ns->pid_allocated =PIDNS_ADDING ; -INIT_WORK (&ns->proc_work, proc_cleanup_work);return ns;kernel/sysctl_binary.c +2
-4@@ -17,6 +17,7 @@#include <linux/uuid.h> #include <linux/slab.h> #include <linux/compat.h> +#include <linux/proc_ns.h> #ifdef CONFIG_SYSCTL_SYSCALL @@ -1278,7 +1279,6 @@ static ssize_t binary_sysctl(const int *name, int nlen, void __user *oldval, size_t oldlen, void __user *newval, size_t newlen) { const struct bin_table *table = NULL; -struct vfsmount *mnt;struct file *file;ssize_t result;char *pathname;@@ -1301,8 +1301,7 @@ static ssize_t binary_sysctl(const int *name, int nlen, goto out_putname; } - mnt =task_active_pid_ns (current)->proc_mnt; - file =file_open_root (mnt->mnt_root, mnt, pathname, flags,0 ); + file =file_open_proc (pathname, flags,0 ); result =PTR_ERR (file);if (IS_ERR (file)) goto out_putname; --2.17.1