[PATCH v3 0/1] Restrict access to TIOCLINUX

[PATCH v3 0/1] Restrict access to TIOCLINUX

[Günther Noack]

Hello!

This is a re-send of a patch by Hanno Böck from 2023-04-02 [1], to restrict the
use of the copy-and-paste functionality in the TIOCLINUX IOCTL.

These copy-and-paste operations can be misused in the same way as the TIOCSTI
IOCTL, which can be disabled with a CONFIG option, since commit 83efeeeb3d04
("tty: Allow TIOCSTI to be disabled") and commit 690c8b804ad2 ("TIOCSTI: always
enable for CAP_SYS_ADMIN").  With this option set to N, the use of TIOCSTI
requires CAP_SYS_ADMIN.

We believe that it should be OK to not make this configurable: For TIOCLINUX's
copy-and-paste subcommands, the only known usage so far is GPM.  I have
personally verified that this continues to work, as GPM runs as root.

The number of affected programs should be much lower than it was the case for
TIOCSTI (as TIOCLINUX only applies to virtual terminals), and even in the
TIOCLINUX case, only a handful of legitimate use cases were mentioned.  (BRLTTY,
tcsh, Emacs, special versions of "mail").  I have high confidence that GPM is
the only existing usage of that copy-and-paste feature.

(If configurability is really required, the way to be absolutely sure would be
to introduce a CONFIG option for it as well -- but it would be a pretty obscure
option to have, but we can do that if needed.)

Changes in v3:
 - Added missing Signed-off-by: line

Changes in v2:
 - Rebased to Linux v6.5
 - Reworded commit message a bit
 - Added Tested-By

[1] https://lore.kernel.org/all/20230402160815.74760f87.hanno@hboeck.de/

Hanno Böck (1):
  tty: Restrict access to TIOCLINUX' copy-and-paste subcommands

 drivers/tty/vt/vt.c | 6 ++++++
 1 file changed, 6 insertions(+)


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
-- 
2.42.0.rc2.253.gd59a3bf2b4-goog

[PATCH v3 1/1] tty: Restrict access to TIOCLINUX' copy-and-paste subcommands

[Günther Noack]

From: Hanno Böck <hanno@hboeck.de>

TIOCLINUX can be used for privilege escalation on virtual terminals when
code is executed via tools like su/sudo and sandboxing tools.

By abusing the selection features, a lower-privileged application can
write content to the console, select and copy/paste that content and
thereby executing code on the privileged account. See also the poc
here:

  https://www.openwall.com/lists/oss-security/2023/03/14/3

Selection is usually used by tools like gpm that provide mouse features
on the virtual console. gpm already runs as root (due to earlier
changes that restrict access to a user on the current TTY), therefore
it will still work with this change.

With this change, the following TIOCLINUX subcommands require
CAP_SYS_ADMIN:

 * TIOCL_SETSEL - setting the selected region on the terminal
 * TIOCL_PASTESEL - pasting the contents of the selected region into
   the input buffer
 * TIOCL_SELLOADLUT - changing word-by-word selection behaviour

The security problem mitigated is similar to the security risks caused
by TIOCSTI, which, since kernel 6.2, can be disabled with
CONFIG_LEGACY_TIOCSTI=n.

Signed-off-by: Hanno Böck <hanno@hboeck.de>
Signed-off-by: Günther Noack <gnoack@google.com>
Tested-by: Günther Noack <gnoack@google.com>
---
 drivers/tty/vt/vt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 1e8e57b45688..1eb30ed1118d 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3156,9 +3156,13 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 
 	switch (type) {
 	case TIOCL_SETSEL:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
 		return set_selection_user((struct tiocl_selection
 					 __user *)(p+1), tty);
 	case TIOCL_PASTESEL:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
 		return paste_selection(tty);
 	case TIOCL_UNBLANKSCREEN:
 		console_lock();
@@ -3166,6 +3170,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 		console_unlock();
 		break;
 	case TIOCL_SELLOADLUT:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
 		console_lock();
 		ret = sel_loadlut(p);
 		console_unlock();
-- 
2.42.0.rc2.253.gd59a3bf2b4-goog

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Samuel Thibault]

Günther Noack, le lun. 28 août 2023 18:41:16 +0200, a ecrit:
> The number of affected programs should be much lower than it was the case for
> TIOCSTI (as TIOCLINUX only applies to virtual terminals), and even in the
> TIOCLINUX case, only a handful of legitimate use cases were mentioned.  (BRLTTY,
> tcsh, Emacs, special versions of "mail").  I have high confidence that GPM is
> the only existing usage of that copy-and-paste feature.

BRLTTY also uses it. It is also admin, so your change is fine :)

FI, https://codesearch.debian.net/ is a very convenient tool to check
what FOSS might be using something.

Samuel

Re: [PATCH v3 1/1] tty: Restrict access to TIOCLINUX' copy-and-paste subcommands

[Mickaël Salaün]

On Mon, Aug 28, 2023 at 06:41:17PM +0200, Günther Noack wrote:
> From: Hanno Böck <hanno@hboeck.de>
> 
> TIOCLINUX can be used for privilege escalation on virtual terminals when
> code is executed via tools like su/sudo and sandboxing tools.
> 
> By abusing the selection features, a lower-privileged application can
> write content to the console, select and copy/paste that content and
> thereby executing code on the privileged account. See also the poc
> here:
> 
>   https://www.openwall.com/lists/oss-security/2023/03/14/3
> 
> Selection is usually used by tools like gpm that provide mouse features
> on the virtual console. gpm already runs as root (due to earlier
> changes that restrict access to a user on the current TTY), therefore
> it will still work with this change.
> 
> With this change, the following TIOCLINUX subcommands require
> CAP_SYS_ADMIN:
> 
>  * TIOCL_SETSEL - setting the selected region on the terminal
>  * TIOCL_PASTESEL - pasting the contents of the selected region into
>    the input buffer
>  * TIOCL_SELLOADLUT - changing word-by-word selection behaviour
> 
> The security problem mitigated is similar to the security risks caused
> by TIOCSTI, which, since kernel 6.2, can be disabled with
> CONFIG_LEGACY_TIOCSTI=n.
> 
> Signed-off-by: Hanno Böck <hanno@hboeck.de>
> Signed-off-by: Günther Noack <gnoack@google.com>

The SoB rules are tricky, you cannot have a Signed-off-by if you are not
in the From/Author or Committer or Co-Developed-by fields:
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

It should be:

Co-Developed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Günther Noack <gnoack@google.com>
Signed-off-by: Hanno Böck <hanno@hboeck.de>

> Tested-by: Günther Noack <gnoack@google.com>

This Tested-by should not be required anymore because of your SoB,
which should implicitly stipulate that you tested this patch.

I'm not sure if it's worth sending another version with only this fix
though, if there is no more issue I guess the maintainer picking it
could fix it.


> ---
>  drivers/tty/vt/vt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 1e8e57b45688..1eb30ed1118d 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3156,9 +3156,13 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
>  
>  	switch (type) {
>  	case TIOCL_SETSEL:
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EPERM;
>  		return set_selection_user((struct tiocl_selection
>  					 __user *)(p+1), tty);
>  	case TIOCL_PASTESEL:
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EPERM;
>  		return paste_selection(tty);
>  	case TIOCL_UNBLANKSCREEN:
>  		console_lock();
> @@ -3166,6 +3170,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
>  		console_unlock();
>  		break;
>  	case TIOCL_SELLOADLUT:
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EPERM;
>  		console_lock();
>  		ret = sel_loadlut(p);
>  		console_unlock();
> -- 
> 2.42.0.rc2.253.gd59a3bf2b4-goog
> 

Re: [PATCH v3 1/1] tty: Restrict access to TIOCLINUX' copy-and-paste subcommands

[Greg KH]

On Mon, Aug 28, 2023 at 08:43:39PM +0200, Mickaël Salaün wrote:
> On Mon, Aug 28, 2023 at 06:41:17PM +0200, Günther Noack wrote:
> > From: Hanno Böck <hanno@hboeck.de>
> > 
> > TIOCLINUX can be used for privilege escalation on virtual terminals when
> > code is executed via tools like su/sudo and sandboxing tools.
> > 
> > By abusing the selection features, a lower-privileged application can
> > write content to the console, select and copy/paste that content and
> > thereby executing code on the privileged account. See also the poc
> > here:
> > 
> >   https://www.openwall.com/lists/oss-security/2023/03/14/3
> > 
> > Selection is usually used by tools like gpm that provide mouse features
> > on the virtual console. gpm already runs as root (due to earlier
> > changes that restrict access to a user on the current TTY), therefore
> > it will still work with this change.
> > 
> > With this change, the following TIOCLINUX subcommands require
> > CAP_SYS_ADMIN:
> > 
> >  * TIOCL_SETSEL - setting the selected region on the terminal
> >  * TIOCL_PASTESEL - pasting the contents of the selected region into
> >    the input buffer
> >  * TIOCL_SELLOADLUT - changing word-by-word selection behaviour
> > 
> > The security problem mitigated is similar to the security risks caused
> > by TIOCSTI, which, since kernel 6.2, can be disabled with
> > CONFIG_LEGACY_TIOCSTI=n.
> > 
> > Signed-off-by: Hanno Böck <hanno@hboeck.de>
> > Signed-off-by: Günther Noack <gnoack@google.com>
> 
> The SoB rules are tricky, you cannot have a Signed-off-by if you are not
> in the From/Author or Committer or Co-Developed-by fields:
> https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Not true at all, maintainers add their signed-off-by to everything they
apply, and you HAVE to add it to a patch that flows through you to
someone else, as per the DCO.

> It should be:
> 
> Co-Developed-by: Günther Noack <gnoack@google.com>

Not if this person was not a developer on it, no.

> Signed-off-by: Günther Noack <gnoack@google.com>
> Signed-off-by: Hanno Böck <hanno@hboeck.de>
> 
> > Tested-by: Günther Noack <gnoack@google.com>
> 
> This Tested-by should not be required anymore because of your SoB,
> which should implicitly stipulate that you tested this patch.
> 
> I'm not sure if it's worth sending another version with only this fix
> though, if there is no more issue I guess the maintainer picking it
> could fix it.

As submitted, it is correct.

thanks,

greg k-h

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Günther Noack]

Hello Samuel!

On Mon, Aug 28, 2023 at 06:45:21PM +0200, Samuel Thibault wrote:
> Günther Noack, le lun. 28 août 2023 18:41:16 +0200, a ecrit:
> BRLTTY also uses it. It is also admin, so your change is fine :)
> 
> FI, https://codesearch.debian.net/ is a very convenient tool to check
> what FOSS might be using something.

Thanks, that is an excellent pointer!

Let me update the list of known usages then: The TIOCL_SETSEL, TIOCL_PASTESEL
and TIOCL_SELLOADLUT mentions found on codesearch.debian.net are:

(1) Actual invocations:

 * consolation:
     "consolation" is a gpm clone, which also runs as root.
     (I have not had the chance to test this one yet.)
 * BRLTTY:
     Uses TIOCL_SETSEL as a means to highlight portions of the screen.
     The TIOCSTI patch made BRLTTY work by requiring CAP_SYS_ADMIN,
     so we know that BRLTTY has that capability (it runs as root and
     does not drop it).

(2) Some irrelevant matches:

 * snapd: has a unit test mentioning it, to test their seccomp filters
 * libexplain: mentions it, but does not call it (it's a library for
   human-readably decoding system calls)
 * manpages: documentation


*Outside* of codesearch.debian.org:

 * gpm:
     I've verified that this works with the patch.
     (To my surprise, Debian does not index this project's code.)

FWIW, I also briefly looked into "jamd" (https://jamd.sourceforge.net/), which
was mentioned as similar in the manpage for "consolation", but that software
does not use any ioctls at all.

So overall, it still seems like nothing should break. 👍

—Günther

-- 
Sent using Mutt 🐕 Woof Woof

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Samuel Thibault]

Günther Noack, le mar. 29 août 2023 15:00:19 +0200, a ecrit:
>  * gpm:
>      I've verified that this works with the patch.
>      (To my surprise, Debian does not index this project's code.)

? it does. But gpm does not use TIOCL_* constants unfortunately...

Samuel

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Günther Noack]

On Tue, Aug 29, 2023 at 03:00:19PM +0200, Günther Noack wrote:
> Let me update the list of known usages then: The TIOCL_SETSEL, TIOCL_PASTESEL
> and TIOCL_SELLOADLUT mentions found on codesearch.debian.net are:
> 
> (1) Actual invocations:
> 
>  * consolation:
>      "consolation" is a gpm clone, which also runs as root.
>      (I have not had the chance to test this one yet.)

I have tested the consolation program with a kernel that has the patch, and it
works as expected -- you can copy and paste on the console.


>  * BRLTTY:
>      Uses TIOCL_SETSEL as a means to highlight portions of the screen.
>      The TIOCSTI patch made BRLTTY work by requiring CAP_SYS_ADMIN,
>      so we know that BRLTTY has that capability (it runs as root and
>      does not drop it).
> 
> (2) Some irrelevant matches:
> 
>  * snapd: has a unit test mentioning it, to test their seccomp filters
>  * libexplain: mentions it, but does not call it (it's a library for
>    human-readably decoding system calls)
>  * manpages: documentation
> 
> 
> *Outside* of codesearch.debian.org:
> 
>  * gpm:
>      I've verified that this works with the patch.
>      (To my surprise, Debian does not index this project's code.)

(As Samuel pointed out, I was wrong there - Debian does index it, but it does
not use the #defines from the headers... who would have thought...)


> FWIW, I also briefly looked into "jamd" (https://jamd.sourceforge.net/), which
> was mentioned as similar in the manpage for "consolation", but that software
> does not use any ioctls at all.
> 
> So overall, it still seems like nothing should break. 👍

Summarizing the above - the only three programs which are known to use the
affected TIOCLINUX subcommands are:

* consolation (tested)
* gpm (tested)
* BRLTTY (known to work with TIOCSTI, where the same CAP_SYS_ADMIN requirement
  is imposed for a while now)

I think that this is a safe change for the existing usages and that we have done
the due diligence required to turn off these features.

Greg, could you please have another look?

Thanks,
—Günther

-- 
Sent using Mutt 🐕 Woof Woof

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Kees Cook]

On Fri, Sep 15, 2023 at 03:32:29PM +0200, Günther Noack wrote:
> On Tue, Aug 29, 2023 at 03:00:19PM +0200, Günther Noack wrote:
> > Let me update the list of known usages then: The TIOCL_SETSEL, TIOCL_PASTESEL
> > and TIOCL_SELLOADLUT mentions found on codesearch.debian.net are:
> > 
> > (1) Actual invocations:
> > 
> >  * consolation:
> >      "consolation" is a gpm clone, which also runs as root.
> >      (I have not had the chance to test this one yet.)
> 
> I have tested the consolation program with a kernel that has the patch, and it
> works as expected -- you can copy and paste on the console.
> 
> 
> >  * BRLTTY:
> >      Uses TIOCL_SETSEL as a means to highlight portions of the screen.
> >      The TIOCSTI patch made BRLTTY work by requiring CAP_SYS_ADMIN,
> >      so we know that BRLTTY has that capability (it runs as root and
> >      does not drop it).
> > 
> > (2) Some irrelevant matches:
> > 
> >  * snapd: has a unit test mentioning it, to test their seccomp filters
> >  * libexplain: mentions it, but does not call it (it's a library for
> >    human-readably decoding system calls)
> >  * manpages: documentation
> > 
> > 
> > *Outside* of codesearch.debian.org:
> > 
> >  * gpm:
> >      I've verified that this works with the patch.
> >      (To my surprise, Debian does not index this project's code.)
> 
> (As Samuel pointed out, I was wrong there - Debian does index it, but it does
> not use the #defines from the headers... who would have thought...)
> 
> 
> > FWIW, I also briefly looked into "jamd" (https://jamd.sourceforge.net/), which
> > was mentioned as similar in the manpage for "consolation", but that software
> > does not use any ioctls at all.
> > 
> > So overall, it still seems like nothing should break. 👍
> 
> Summarizing the above - the only three programs which are known to use the
> affected TIOCLINUX subcommands are:
> 
> * consolation (tested)
> * gpm (tested)
> * BRLTTY (known to work with TIOCSTI, where the same CAP_SYS_ADMIN requirement
>   is imposed for a while now)
> 
> I think that this is a safe change for the existing usages and that we have done
> the due diligence required to turn off these features.
> 
> Greg, could you please have another look?

Can you spin a v4 with all these details collected into the commit log?
That should be sufficient information for Greg, I would think.

Thanks for checking each of these!

-Kees

-- 
Kees Cook

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Greg KH]

On Mon, Oct 09, 2023 at 01:19:47PM -0700, Kees Cook wrote:
> On Fri, Sep 15, 2023 at 03:32:29PM +0200, Günther Noack wrote:
> > On Tue, Aug 29, 2023 at 03:00:19PM +0200, Günther Noack wrote:
> > > Let me update the list of known usages then: The TIOCL_SETSEL, TIOCL_PASTESEL
> > > and TIOCL_SELLOADLUT mentions found on codesearch.debian.net are:
> > > 
> > > (1) Actual invocations:
> > > 
> > >  * consolation:
> > >      "consolation" is a gpm clone, which also runs as root.
> > >      (I have not had the chance to test this one yet.)
> > 
> > I have tested the consolation program with a kernel that has the patch, and it
> > works as expected -- you can copy and paste on the console.
> > 
> > 
> > >  * BRLTTY:
> > >      Uses TIOCL_SETSEL as a means to highlight portions of the screen.
> > >      The TIOCSTI patch made BRLTTY work by requiring CAP_SYS_ADMIN,
> > >      so we know that BRLTTY has that capability (it runs as root and
> > >      does not drop it).
> > > 
> > > (2) Some irrelevant matches:
> > > 
> > >  * snapd: has a unit test mentioning it, to test their seccomp filters
> > >  * libexplain: mentions it, but does not call it (it's a library for
> > >    human-readably decoding system calls)
> > >  * manpages: documentation
> > > 
> > > 
> > > *Outside* of codesearch.debian.org:
> > > 
> > >  * gpm:
> > >      I've verified that this works with the patch.
> > >      (To my surprise, Debian does not index this project's code.)
> > 
> > (As Samuel pointed out, I was wrong there - Debian does index it, but it does
> > not use the #defines from the headers... who would have thought...)
> > 
> > 
> > > FWIW, I also briefly looked into "jamd" (https://jamd.sourceforge.net/), which
> > > was mentioned as similar in the manpage for "consolation", but that software
> > > does not use any ioctls at all.
> > > 
> > > So overall, it still seems like nothing should break. 👍
> > 
> > Summarizing the above - the only three programs which are known to use the
> > affected TIOCLINUX subcommands are:
> > 
> > * consolation (tested)
> > * gpm (tested)
> > * BRLTTY (known to work with TIOCSTI, where the same CAP_SYS_ADMIN requirement
> >   is imposed for a while now)
> > 
> > I think that this is a safe change for the existing usages and that we have done
> > the due diligence required to turn off these features.
> > 
> > Greg, could you please have another look?
> 
> Can you spin a v4 with all these details collected into the commit log?
> That should be sufficient information for Greg, I would think.

This is already commit 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX'
copy-and-paste subcommands") in my tty-next tree, and in linux-next.
It's been there for 5 days now :)

thanks,

greg k-h

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Kees Cook]

On Tue, Oct 10, 2023 at 08:17:42AM +0200, Greg KH wrote:
> On Mon, Oct 09, 2023 at 01:19:47PM -0700, Kees Cook wrote:
> > On Fri, Sep 15, 2023 at 03:32:29PM +0200, Günther Noack wrote:
> > > On Tue, Aug 29, 2023 at 03:00:19PM +0200, Günther Noack wrote:
> > > > Let me update the list of known usages then: The TIOCL_SETSEL, TIOCL_PASTESEL
> > > > and TIOCL_SELLOADLUT mentions found on codesearch.debian.net are:
> > > > 
> > > > (1) Actual invocations:
> > > > 
> > > >  * consolation:
> > > >      "consolation" is a gpm clone, which also runs as root.
> > > >      (I have not had the chance to test this one yet.)
> > > 
> > > I have tested the consolation program with a kernel that has the patch, and it
> > > works as expected -- you can copy and paste on the console.
> > > 
> > > 
> > > >  * BRLTTY:
> > > >      Uses TIOCL_SETSEL as a means to highlight portions of the screen.
> > > >      The TIOCSTI patch made BRLTTY work by requiring CAP_SYS_ADMIN,
> > > >      so we know that BRLTTY has that capability (it runs as root and
> > > >      does not drop it).
> > > > 
> > > > (2) Some irrelevant matches:
> > > > 
> > > >  * snapd: has a unit test mentioning it, to test their seccomp filters
> > > >  * libexplain: mentions it, but does not call it (it's a library for
> > > >    human-readably decoding system calls)
> > > >  * manpages: documentation
> > > > 
> > > > 
> > > > *Outside* of codesearch.debian.org:
> > > > 
> > > >  * gpm:
> > > >      I've verified that this works with the patch.
> > > >      (To my surprise, Debian does not index this project's code.)
> > > 
> > > (As Samuel pointed out, I was wrong there - Debian does index it, but it does
> > > not use the #defines from the headers... who would have thought...)
> > > 
> > > 
> > > > FWIW, I also briefly looked into "jamd" (https://jamd.sourceforge.net/), which
> > > > was mentioned as similar in the manpage for "consolation", but that software
> > > > does not use any ioctls at all.
> > > > 
> > > > So overall, it still seems like nothing should break. 👍
> > > 
> > > Summarizing the above - the only three programs which are known to use the
> > > affected TIOCLINUX subcommands are:
> > > 
> > > * consolation (tested)
> > > * gpm (tested)
> > > * BRLTTY (known to work with TIOCSTI, where the same CAP_SYS_ADMIN requirement
> > >   is imposed for a while now)
> > > 
> > > I think that this is a safe change for the existing usages and that we have done
> > > the due diligence required to turn off these features.
> > > 
> > > Greg, could you please have another look?
> > 
> > Can you spin a v4 with all these details collected into the commit log?
> > That should be sufficient information for Greg, I would think.
> 
> This is already commit 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX'
> copy-and-paste subcommands") in my tty-next tree, and in linux-next.
> It's been there for 5 days now :)

Oh perfect! Thanks.

On a related topic, I wonder if you can change your scripting to reply
to the original patch thread when something lands? This is helpful when
going back over old threads, etc. (This is what the various other bots
and b4 tooling does...)

-Kees

-- 
Kees Cook

Re: [PATCH v3 0/1] Restrict access to TIOCLINUX

[Greg KH]

On Tue, Oct 10, 2023 at 03:23:42PM -0700, Kees Cook wrote:
> On Tue, Oct 10, 2023 at 08:17:42AM +0200, Greg KH wrote:
> > On Mon, Oct 09, 2023 at 01:19:47PM -0700, Kees Cook wrote:
> > > On Fri, Sep 15, 2023 at 03:32:29PM +0200, Günther Noack wrote:
> > > > On Tue, Aug 29, 2023 at 03:00:19PM +0200, Günther Noack wrote:
> > > > > Let me update the list of known usages then: The TIOCL_SETSEL, TIOCL_PASTESEL
> > > > > and TIOCL_SELLOADLUT mentions found on codesearch.debian.net are:
> > > > > 
> > > > > (1) Actual invocations:
> > > > > 
> > > > >  * consolation:
> > > > >      "consolation" is a gpm clone, which also runs as root.
> > > > >      (I have not had the chance to test this one yet.)
> > > > 
> > > > I have tested the consolation program with a kernel that has the patch, and it
> > > > works as expected -- you can copy and paste on the console.
> > > > 
> > > > 
> > > > >  * BRLTTY:
> > > > >      Uses TIOCL_SETSEL as a means to highlight portions of the screen.
> > > > >      The TIOCSTI patch made BRLTTY work by requiring CAP_SYS_ADMIN,
> > > > >      so we know that BRLTTY has that capability (it runs as root and
> > > > >      does not drop it).
> > > > > 
> > > > > (2) Some irrelevant matches:
> > > > > 
> > > > >  * snapd: has a unit test mentioning it, to test their seccomp filters
> > > > >  * libexplain: mentions it, but does not call it (it's a library for
> > > > >    human-readably decoding system calls)
> > > > >  * manpages: documentation
> > > > > 
> > > > > 
> > > > > *Outside* of codesearch.debian.org:
> > > > > 
> > > > >  * gpm:
> > > > >      I've verified that this works with the patch.
> > > > >      (To my surprise, Debian does not index this project's code.)
> > > > 
> > > > (As Samuel pointed out, I was wrong there - Debian does index it, but it does
> > > > not use the #defines from the headers... who would have thought...)
> > > > 
> > > > 
> > > > > FWIW, I also briefly looked into "jamd" (https://jamd.sourceforge.net/), which
> > > > > was mentioned as similar in the manpage for "consolation", but that software
> > > > > does not use any ioctls at all.
> > > > > 
> > > > > So overall, it still seems like nothing should break. 👍
> > > > 
> > > > Summarizing the above - the only three programs which are known to use the
> > > > affected TIOCLINUX subcommands are:
> > > > 
> > > > * consolation (tested)
> > > > * gpm (tested)
> > > > * BRLTTY (known to work with TIOCSTI, where the same CAP_SYS_ADMIN requirement
> > > >   is imposed for a while now)
> > > > 
> > > > I think that this is a safe change for the existing usages and that we have done
> > > > the due diligence required to turn off these features.
> > > > 
> > > > Greg, could you please have another look?
> > > 
> > > Can you spin a v4 with all these details collected into the commit log?
> > > That should be sufficient information for Greg, I would think.
> > 
> > This is already commit 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX'
> > copy-and-paste subcommands") in my tty-next tree, and in linux-next.
> > It's been there for 5 days now :)
> 
> Oh perfect! Thanks.
> 
> On a related topic, I wonder if you can change your scripting to reply
> to the original patch thread when something lands? This is helpful when
> going back over old threads, etc. (This is what the various other bots
> and b4 tooling does...)

My script picks the emails out of the patch that is committed, it
doesn't know anything about the original email anymore (as sometimes it
is days later when it propagates to a non-rebasable-branch).  I use b4
to suck down the patch originally and it will add all of the people who
reviewed it or gave any other "tag" to it.

What b4 option does a "I applied this patch" response?  The
--cc-trailers option to 'shazam'?  Or something else?

thanks,

greg k-h

sending commit notification to patch thread (was "Re: [PATCH v3 0/1] Restrict access to TIOCLINUX")

[Kees Cook]

On Wed, Oct 11, 2023 at 08:22:49AM +0200, Greg KH wrote:
> What b4 option does a "I applied this patch" response?  The
> --cc-trailers option to 'shazam'?  Or something else?

If you're using "b4 shazam", then it'll keep a record of it already and
you can use "b4 ty" to send the "thank you" email to the thread.

-- 
Kees Cook