linux-kernel - Re: [PATCH] tty: replace capable() with file_ns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH4c4j+BEJqMqECPDgOF5vq4hg7_yBRrLBPSuTKnA+CO658SOQ@mail.gmail.com>
Date: Wed, 18 Jun 2025 12:02:28 +0530
From: Pranav Tyagi <pranav.tyagi03@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, kees@...nel.org, 
	skhan@...uxfoundation.org, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH] tty: replace capable() with file_ns_capable()

On Tue, Jun 17, 2025 at 5:34 PM David Laight
<david.laight.linux@...il.com> wrote:
>
> On Sat,  7 Jun 2025 19:11:14 +0530
> Pranav Tyagi <pranav.tyagi03@...il.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@...il.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;
>