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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 20 Oct 2022 08:05:05 +0200 From: Jiri Slaby <jirislaby@...nel.org> To: Kees Cook <keescook@...omium.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org> Cc: Simon Brand <simon.brand@...tadigitale.de>, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH v2 2/2] tty: Allow TIOCSTI to be disabled Hi, On 15. 10. 22, 8:45, Kees Cook wrote: > TIOCSTI continues its long history of being used in privilege escalation > attacks[1]. Prior attempts to provide a mechanism to disable this have > devolved into discussions around creating full-blown LSMs to provide > arbitrary ioctl filtering, which is hugely over-engineered -- only > TIOCSTI is being used this way. 3 years ago OpenBSD entirely removed > TIOCSTI[2], Android has had it filtered for longer[3], and the tools that > had historically used TIOCSTI either do not need it, are not commonly > built with it, or have had its use removed. > > Provide a simple CONFIG and global sysctl to disable this for the system > builders who have wanted this functionality for literally decades now, > much like the ldisc_autoload CONFIG and sysctl. > > [1] https://lore.kernel.org/linux-hardening/Y0m9l52AKmw6Yxi1@hostpad > [2] https://undeadly.org/cgi?action=article;sid=20170701132619 > [3] https://lore.kernel.org/lkml/CAFJ0LnFGRuEEn1tCLhoki8ZyWrKfktbF+rwwN7WzyC_kBFoQVA@mail.gmail.com/ > > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org> > Cc: Jiri Slaby <jirislaby@...nel.org> > Cc: Simon Brand <simon.brand@...tadigitale.de> > Signed-off-by: Kees Cook <keescook@...omium.org> ... > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -2275,11 +2275,15 @@ static int tty_fasync(int fd, struct file *filp, int on) > * * Called functions take tty_ldiscs_lock > * * current->signal->tty check is safe without locks > */ > +static int tty_legacy_tiocsti __read_mostly = IS_BUILTIN(CONFIG_LEGACY_TIOCSTI); This can be bool, right? And IS_ENABLED() sounds more appropriate here. > static int tiocsti(struct tty_struct *tty, char __user *p) > { > char ch, mbz = 0; > struct tty_ldisc *ld; > > + if (!tty_legacy_tiocsti) > + return -EIO; > + > if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN)) > return -EPERM; > if (get_user(ch, p)) > @@ -3582,6 +3586,15 @@ void console_sysfs_notify(void) > } > > static struct ctl_table tty_table[] = { > + { > + .procname = "legacy_tiocsti", > + .data = &tty_legacy_tiocsti, > + .maxlen = sizeof(tty_legacy_tiocsti), > + .mode = 0644, > + .proc_handler = proc_dointvec, > + .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_ONE, Then this becomes just proc_dobool without extras. Or we can leave it as int, allow 0, 1, and 2. 2 would log_limited the caller's comm before EIO. Just thinking loudly. Maybe the EIO is enough for users to notice. Likely… thanks, -- js suse labs
Powered by blists - more mailing lists