Messages in this thread | | | Date | Sat, 16 Nov 2013 17:03:34 +0100 | From | Oleg Nesterov <> | Subject | Re: [patch 1/2] autofs4: allow autofs to work outside the initial PID namespace |
| |
On 11/15, Andrew Morton wrote: > > Enable autofs4 to work in a "container". oz_pgrp is converted from pid_t > to struct pid and this is stored at mount time based on the "pgrp=" option > or if the option is missing then the current pgrp.
I don't understand this code, so I am probably wrong. And this is minor anyway, but...
> @@ -357,7 +358,17 @@ static int autofs_dev_ioctl_setpipefd(st > mutex_unlock(&sbi->wq_mutex); > return -EBUSY; > } else { > - struct file *pipe = fget(pipefd); > + struct file *pipe; > + > + new_pid = get_task_pid(current, PIDTYPE_PGID); > + > + if (ns_of_pid(new_pid) != ns_of_pid(sbi->oz_pgrp)) { > + AUTOFS_WARN("Not allowed to change PID namespace"); > + err = -EINVAL; > + goto out; > + } > + > + pipe = fget(pipefd); > if (!pipe) { > err = -EBADF; > goto out; > @@ -367,12 +378,13 @@ static int autofs_dev_ioctl_setpipefd(st > fput(pipe); > goto out; > } > - sbi->oz_pgrp = task_pgrp_nr(current); > + swap(sbi->oz_pgrp, new_pid); > sbi->pipefd = pipefd; > sbi->pipe = pipe; > sbi->catatonic = 0; > } > out: > + put_pid(new_pid);
This looks suspicious, put_pid() can actually kfree the old sbi->oz_pgrp swapped above. IOW, this assumes we can't race with any user of ->oz_pgrp.
For example,
> @@ -80,7 +83,7 @@ static int autofs4_show_options(struct s > if (!gid_eq(root_inode->i_gid, GLOBAL_ROOT_GID)) > seq_printf(m, ",gid=%u", > from_kgid_munged(&init_user_ns, root_inode->i_gid)); > - seq_printf(m, ",pgrp=%d", sbi->oz_pgrp); > + seq_printf(m, ",pgrp=%d", pid_vnr(sbi->oz_pgrp));
Can't this race with autofs_dev_ioctl_setpipefd() above?
Oleg.
| |