Dwight Engen <dwight.en...@oracle.com> writes:
> Hi Eric, any chance you've had a moment to mull this over any more?
> I've also CC'ed Jiri since he is listed in MAINTAINERS for the TTY
> layer :)
Honestly I really haven't.
For the most part I have been in feature freeze and bug fix mode.
It gets tricky getting the permission checks correct on something like
this and I can easily imagine more people being able to call
vhangup/revoke without care could easily translate into a DDOS attack.
Say revoking the file descriptor that logs system messages out the
serial console.
I think this problem would be easier if we had a devpts specific
solution as the concept of owner of a device node is much clearer there.
There is only one device node.
I would really like to see a combination of implementation and reasoning
that doesn't leave corner cases for me to worry about what the bad guys
are going to do with it.
Eric
> On Wed, 6 Mar 2013 09:58:53 -0600
> Serge Hallyn <serge.hal...@ubuntu.com> wrote:
>
>> Quoting Dwight Engen (dwight.en...@oracle.com):
>> > On Mon, 25 Feb 2013 20:26:21 -0800
>> > ebied...@xmission.com (Eric W. Biederman) wrote:
>> ...
>> > > For pty's since they only have the single device node. We can
>> > > probably do kuid_has_mapping and kgid_has_mapping to see if we
>> > > should have super user privileges over the pty. But that is
>> > > specific to ptys on /dev/pts. Normal devices potentially can have
>> > > device nodes with different permissions in different places so we
>> > > can't figure out an owner for the device.
>> > >
>> > > Eric
>> > >
>> >
>> > Yeah, I agree we don't want something pty specific. The following
>> > patch assumes we define a tty as belonging to the user ns of its
>> > session leader (and if it doesn't have one then to init_user_ns) as
>> > you first suggested. I added locking which avoids the race with
>> > disassociate_tty() and ensures the user ns doesn't get unrefed by
>> > way of put_pid(tty->session).
>> >
>> > Note that I think TIOCVHANGUP should be checking for
>> > CAP_SYS_TTY_CONFIG instead of CAP_SYS_ADMIN to be consistent with
>> > vhangup(2), but I did not change that in the refactoring here.
>> >
>> > --
>> >
>> > From 697f842ffc709312e5775e3d1d0782079c3070dc Mon Sep 17 00:00:00
>> > 2001 From: Dwight Engen <dwight.en...@oracle.com>
>> > Date: Fri, 1 Mar 2013 13:49:58 -0500
>> > Subject: [PATCH] make vhangup and TIOCVHANGUP namespace aware
>> >
>> > Signed-off-by: Dwight Engen <dwight.en...@oracle.com>
>>
>> It looks good to me. Eric?
>>
>> Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>
>>
>> > ---
>> > drivers/tty/tty_io.c | 36 ++++++++++++++++++++++++++++++------
>> > fs/open.c | 6 +-----
>> > include/linux/tty.h | 2 +-
>> > 3 files changed, 32 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> > index a057db8..764d4e7 100644
>> > --- a/drivers/tty/tty_io.c
>> > +++ b/drivers/tty/tty_io.c
>> > @@ -104,6 +104,7 @@
>> >
>> > #include <linux/kmod.h>
>> > #include <linux/nsproxy.h>
>> > +#include <linux/pid_namespace.h>
>> >
>> > #undef TTY_DEBUG_HANGUP
>> >
>> > @@ -722,6 +723,29 @@ void tty_vhangup(struct tty_struct *tty)
>> >
>> > EXPORT_SYMBOL(tty_vhangup);
>> >
>> > +/**
>> > + * tty_vhangup_check_cap - process vhangup
>> > checking for capablity
>> > + *
>> > + * Perform a vhangup on the given tty
>> > + */
>> > +
>> > +static int tty_vhangup_check_cap(struct tty_struct *tty, int cap)
>> > +{
>> > + unsigned long flags;
>> > + int retval = 0;
>> > + struct user_namespace *ns = &init_user_ns;
>> > +
>> > + spin_lock_irqsave(&tty->ctrl_lock, flags);
>> > + if (tty->session)
>> > + ns = ns_of_pid(tty->session)->user_ns;
>> > + if (!ns_capable(ns, cap))
>> > + retval = -EPERM;
>> > + spin_unlock_irqrestore(&tty->ctrl_lock, flags);
>> > +
>> > + if (!retval)
>> > + tty_vhangup(tty);
>> > + return retval;
>> > +}
>> >
>> > /**
>> > * tty_vhangup_self - process vhangup for own
>> > ctty @@ -729,15 +753,18 @@ EXPORT_SYMBOL(tty_vhangup);
>> > * Perform a vhangup on the current controlling tty
>> > */
>> >
>> > -void tty_vhangup_self(void)
>> > +int tty_vhangup_self(void)
>> > {
>> > struct tty_struct *tty;
>> > + int retval = 0;
>> >
>> > tty = get_current_tty();
>> > if (tty) {
>> > - tty_vhangup(tty);
>> > + retval = tty_vhangup_check_cap(tty,
>> > CAP_SYS_TTY_CONFIG); tty_kref_put(tty);
>> > }
>> > +
>> > + return retval;
>> > }
>> >
>> > /**
>> > @@ -2710,10 +2737,7 @@ long tty_ioctl(struct file *file, unsigned
>> > int cmd, unsigned long arg) case TIOCSETD:
>> > return tiocsetd(tty, p);
>> > case TIOCVHANGUP:
>> > - if (!capable(CAP_SYS_ADMIN))
>> > - return -EPERM;
>> > - tty_vhangup(tty);
>> > - return 0;
>> > + return tty_vhangup_check_cap(tty, CAP_SYS_ADMIN);
>> > case TIOCGDEV:
>> > {
>> > unsigned int ret =
>> > new_encode_dev(tty_devnum(real_tty)); diff --git a/fs/open.c
>> > b/fs/open.c index 9b33c0c..19ac16e 100644
>> > --- a/fs/open.c
>> > +++ b/fs/open.c
>> > @@ -1059,11 +1059,7 @@ EXPORT_SYMBOL(sys_close);
>> > */
>> > SYSCALL_DEFINE0(vhangup)
>> > {
>> > - if (capable(CAP_SYS_TTY_CONFIG)) {
>> > - tty_vhangup_self();
>> > - return 0;
>> > - }
>> > - return -EPERM;
>> > + return tty_vhangup_self();
>> > }
>> >
>> > /*
>> > diff --git a/include/linux/tty.h b/include/linux/tty.h
>> > index 8db1b56..c9d0e9c 100644
>> > --- a/include/linux/tty.h
>> > +++ b/include/linux/tty.h
>> > @@ -379,7 +379,7 @@ extern int tty_signal(int sig, struct
>> > tty_struct *tty); extern void tty_hangup(struct tty_struct *tty);
>> > extern void tty_vhangup(struct tty_struct *tty);
>> > extern void tty_vhangup_locked(struct tty_struct *tty);
>> > -extern void tty_vhangup_self(void);
>> > +extern int tty_vhangup_self(void);
>> > extern void tty_unhangup(struct file *filp);
>> > extern int tty_hung_up_p(struct file *filp);
>> > extern void do_SAK(struct tty_struct *tty);
>> > --
>> > 1.7.12.3
>> >
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
