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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
