Seth Forshee <[email protected]> writes: > root is allowed to steal ttys from other sessions, but it > requires system-wide CAP_SYS_ADMIN and therefore is not possible > for root within a user namespace. This should be allowed so long > as the process doing the stealing is privileged towards the > session leader which currently owns the tty. > > Update the tty code to only require CAP_SYS_ADMIN in the > namespace of the target session leader when stealing a tty. Fall > back to using init_user_ns to preserve the existing behavior for > system-wide root. > > Cc: [email protected] # 3.8+
This is not a regression of any form, nor is it obviously correct so this does not count as a stable material. > Cc: Serge Hallyn <[email protected]> > Cc: "Eric W. Biederman" <[email protected]> > Signed-off-by: Seth Forshee <[email protected]> > --- > drivers/tty/tty_io.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > index c74a00a..1c47f16 100644 > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -2410,7 +2410,19 @@ static int tiocsctty(struct tty_struct *tty, int arg) > * This tty is already the controlling > * tty for another session group! > */ > - if (arg == 1 && capable(CAP_SYS_ADMIN)) { > + struct user_namespace *ns = &init_user_ns; > + struct task_struct *p; > + > + read_lock(&tasklist_lock); > + do_each_pid_task(tty->session, PIDTYPE_SID, p) { > + if (p->signal->leader) { > + ns = task_cred_xxx(p, user_ns); > + break; > + } > + } while_each_pid_task(tty->session, PIDTYPE_SID, p); > + read_unlock(&tasklist_lock); Ugh. That appears to be both racy (what protects the user_ns from going away?) and a possibly allowing revoking a tty from a more privileged processes tty. However I do see a form that can easily verify we won't revoke a tty from a more privileged process. if (arg == 1) { struct user_namespace *user_ns; read_lock(&tasklist_lock); do_each_pid_task(tty->session, PIDTYPE_SID, p) { rcu_read_lock(); user_ns = task_cred_xxx(p, user_ns); if (!ns_capable(user_ns, CAP_SYS_ADMIN)) { rcu_read_unlock(); read_unlock(&task_list_lock); ret = -EPERM; goto out_unlock; } rcu_read_unlock(); } /* Don't drop the the tasklist_lock before * stealing the tasks or the set of tasks can * change, and we only have permission for this set * of tasks. */ /* * Steal it away */ session_clear_tty(tty->session); read_unlock(&task_list_lock); } else { ret = -EPERM; goto out_unlock; } My code above is ugly and could use some cleaning up but it should be correct with respect to this issue. Eric > + if (arg == 1 && ns_capable(user_ns, CAP_SYS_ADMIN)) { > /* > * Steal it away > */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
