| Message ID | 20250607134114.21899-1-pranav.tyagi03@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | tty: replace capable() with file_ns_capable() | expand |
On Sat, Jun 07, 2025 at 07:11:14PM +0530, Pranav Tyagi 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. > > 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)) As you now are affecting the user/kernel api here, how was this tested and are you _SURE_ this is the correct thing to be doing? Did you audit all userspace users of this ioctl that you can find (i.e. a Debian code search) to verify that they can handle this change in behaviour? I need a lot of assurances before being able to take a change like this, for obvious reasons. thanks, greg k-h
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;
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. 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(-)