Re: [PATCH] tty: replace capable() with file_ns_capable()

[Pranav Tyagi <pranav.tyagi03@gmail.com>]

On Tue, Jun 17, 2025 at 5:34 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sat,  7 Jun 2025 19:11:14 +0530
> Pranav Tyagi <pranav.tyagi03@gmail.com> wrote:
>
> > The TIOCCONS ioctl currently uses capable(CAP_SYS_ADMIN) to check for
> > privileges, which validates the current task's credentials. Since this
> > ioctl acts on an open file descriptor, the check should instead use the
> > file opener's credentials.
>
> Is that right?
> A terminal will have been opened before the login sequence changed the user id.
>
> The 'best practise' might be to check both!
>
>         David

Hi,

You're right — I hadn’t fully considered that the terminal is typically
opened before the user ID changes during login. Checking only the file
opener's credentials may miss important security context.

Best practice would indeed be to validate both: ensure that the current
task has sufficient privileges and that the file was opened by an
authorized user.

Thanks for pointing this out — I’ll revise the patch accordingly.

Regards
Pranav Tyagi


>
> >
> > Replace capable() with file_ns_capable() to ensure the capability is
> > checked against file->f_cred in the correct user namespace. This
> > prevents unintended privilege escalation and aligns with best practices
> > for secure ioctl implementations.
> >
> > Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> > Link: https://github.com/KSPP/linux/issues/156
> > ---
> >  drivers/tty/tty_io.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > index e2d92cf70eb7..ee0df35d65c3 100644
> > --- a/drivers/tty/tty_io.c
> > +++ b/drivers/tty/tty_io.c
> > @@ -102,6 +102,9 @@
> >  #include <linux/uaccess.h>
> >  #include <linux/termios_internal.h>
> >  #include <linux/fs.h>
> > +#include <linux/cred.h>
> > +#include <linux/user_namespace.h>
> > +#include <linux/capability.h>
> >
> >  #include <linux/kbd_kern.h>
> >  #include <linux/vt_kern.h>
> > @@ -2379,7 +2382,7 @@ static int tiocswinsz(struct tty_struct *tty, struct winsize __user *arg)
> >   */
> >  static int tioccons(struct file *file)
> >  {
> > -     if (!capable(CAP_SYS_ADMIN))
> > +     if (!file_ns_capable(file, file->f_cred->user_ns, CAP_SYS_ADMIN))
> >               return -EPERM;
> >       if (file->f_op->write_iter == redirected_tty_write) {
> >               struct file *f;
>