With this, processes without CAP_SYS_ADMIN are able to use TIOCLINUX with
subcode TIOCL_SETSEL, in the selection modes TIOCL_SETPOINTER,
TIOCL_SELCLEAR and TIOCL_SELMOUSEREPORT.

TIOCL_SETSEL was previously changed to require CAP_SYS_ADMIN, as this IOCTL
let callers change the selection buffer and could be used to simulate
keypresses.  These three TIOCL_SETSEL selection modes, however, are safe to
use, as they do not modify the selection buffer.

This fixes a mouse support regression that affected Emacs (invisible mouse
cursor).

Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/ee3ec63269b43b34e1c90dd8c9743bf8@finder.org
Fixes: 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands")
Signed-off-by: Günther Noack <gnoack@google.com>
---
Changes in V2:

 * Removed comment in vt.c (per Greg's suggestion)

 * CC'd stable@

 * I *kept* the CAP_SYS_ADMIN check *after* copy_from_user(),
   with the reasoning that:

   1. I do not see a good alternative to reorder the code here.
      We need the data from copy_from_user() in order to know whether
      the CAP_SYS_ADMIN check even needs to be performed.
   2. A previous get_user() from an adjacent memory region already worked
      (making this a very unlikely failure)

I would still appreciate a more formal Tested-by from Hanno (hint, hint) :)

---
 drivers/tty/vt/selection.c | 14 ++++++++++++++
 drivers/tty/vt/vt.c        |  2 --
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 564341f1a74f..0bd6544e30a6 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -192,6 +192,20 @@ int set_selection_user(const struct tiocl_selection __user *sel,
 	if (copy_from_user(&v, sel, sizeof(*sel)))
 		return -EFAULT;
 
+	/*
+	 * TIOCL_SELCLEAR, TIOCL_SELPOINTER and TIOCL_SELMOUSEREPORT are OK to
+	 * use without CAP_SYS_ADMIN as they do not modify the selection.
+	 */
+	switch (v.sel_mode) {
+	case TIOCL_SELCLEAR:
+	case TIOCL_SELPOINTER:
+	case TIOCL_SELMOUSEREPORT:
+		break;
+	default:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+	}
+
 	return set_selection_kernel(&v, tty);
 }
 
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 96842ce817af..be5564ed8c01 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3345,8 +3345,6 @@ 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(param, tty);
 	case TIOCL_PASTESEL:
 		if (!capable(CAP_SYS_ADMIN))
-- 
2.47.1.613.gc27f4b7a9f-goog

On Fri, Jan 10, 2025 at 02:21:22PM +0000, Günther Noack wrote:
> With this, processes without CAP_SYS_ADMIN are able to use TIOCLINUX with
> subcode TIOCL_SETSEL, in the selection modes TIOCL_SETPOINTER,
> TIOCL_SELCLEAR and TIOCL_SELMOUSEREPORT.
> 
> TIOCL_SETSEL was previously changed to require CAP_SYS_ADMIN, as this IOCTL
> let callers change the selection buffer and could be used to simulate
> keypresses.  These three TIOCL_SETSEL selection modes, however, are safe to
> use, as they do not modify the selection buffer.
> 
> This fixes a mouse support regression that affected Emacs (invisible mouse
> cursor).
> 
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/ee3ec63269b43b34e1c90dd8c9743bf8@finder.org
> Fixes: 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands")
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> Changes in V2:
> 
>  * Removed comment in vt.c (per Greg's suggestion)
> 
>  * CC'd stable@
> 
>  * I *kept* the CAP_SYS_ADMIN check *after* copy_from_user(),
>    with the reasoning that:
> 
>    1. I do not see a good alternative to reorder the code here.
>       We need the data from copy_from_user() in order to know whether
>       the CAP_SYS_ADMIN check even needs to be performed.
>    2. A previous get_user() from an adjacent memory region already worked
>       (making this a very unlikely failure)
> 
> I would still appreciate a more formal Tested-by from Hanno (hint, hint) :)

This really is v3, as you sent a v2 last year, right?

b4 got really confused, but I think I figured it out.  Whenever you
resend, bump the version number please, otherwise it causes problems.
Remember, some of use deal with thousands of patches a week...

thanks,

greg k-h

On Fri, Jan 10, 2025 at 02:21:22PM +0000, Günther Noack wrote:
> With this, processes without CAP_SYS_ADMIN are able to use TIOCLINUX with
> subcode TIOCL_SETSEL, in the selection modes TIOCL_SETPOINTER,
> TIOCL_SELCLEAR and TIOCL_SELMOUSEREPORT.
> 
> TIOCL_SETSEL was previously changed to require CAP_SYS_ADMIN, as this IOCTL
> let callers change the selection buffer and could be used to simulate
> keypresses.  These three TIOCL_SETSEL selection modes, however, are safe to
> use, as they do not modify the selection buffer.
> 
> This fixes a mouse support regression that affected Emacs (invisible mouse
> cursor).
> 
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/ee3ec63269b43b34e1c90dd8c9743bf8@finder.org
> Fixes: 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands")
> Signed-off-by: Günther Noack <gnoack@google.com>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Hi, I'm the original reporter of this regression (noticed because it 
impacted GNU Emacs) and I'm wondering if there's any traction on 
creating an updated patch? This thread appears to have stalled out. I 
haven't seen any reply for three weeks.

   -- MJF

Hello Jared and Hanno!

On Sat, Feb 08, 2025 at 07:18:22AM -0800, Jared Finder wrote:
> Hi, I'm the original reporter of this regression (noticed because it
> impacted GNU Emacs) and I'm wondering if there's any traction on creating an
> updated patch? This thread appears to have stalled out. I haven't seen any
> reply for three weeks.
> 
>   -- MJF

Jared, can you please confirm whether Emacs works now with this patch
in the kernel?

I am asking this because I realized that the patch had a bug.  We are
erring in the "secure" direction, but not all TIOCL_SELMOUSEREPORT
invocations work without CAP_SYS_ADMIN.

(TIOCL_SELMOUSEREPORT has to be put into the sel_mode field of the
argument struct, and that field looked like an enum to me, but as it
turns out, the TIOCL_SELMOUSEREPORT is 16 and the lower 4 bits of that
integer are used as an additional argument to indicate the mouse
button status and modifier keys.  I had overlooked that the
implementation was doing this.  As a result, TIOCL_SELMOUSEREPORT
works without CAP_SYS_ADMIN, but only if all four lower bits are 0.)

So, I apologize for the oversight.  -- Jared, can you please confirm
whether TIOCL_SELMOUSEREPORT is called directly from Emacs (rather
than from gpm)?  I tried to trace it with perf but could not reproduce
a scenario where Emacs called it.

If this specific selection mode is not needed by Emacs, I think *the
best thing would be to keep it guarded by CAP_SYS_ADMIN, after all*.

As it turns out, the following scenario is possible:

* A root shell invokes malicious program P and changes its UID to a
  less privileged user, but it passes a FD to the TTY.  (Instead of
  UID switch, it can also be a sandboxed program or another way of
  lowering privileges.)
* Program P enables mouse tracking mode by writing "\033[?1000h".
* Program P sends IOCTL TIOCLINUX with TIOCL_SETSEL with
  TIOCL_SELMOUSEREPORT and passes suitable mouse coordinate and button
  press arguments.  As a response, the terminal writes the escape
  sequence \033[MBXY, where B, X and Y are bytes indicating the button
  press mask and the 1-based mouse X and Y coordinates, all added to
  0x20 (space).

It is an obscure scenario and probably requires a console with a
character width and height above 256 (to control the full byte range),
but it seems that this can in principle be used to simulate short
keyboard inputs to programs (like bash) that are not expecting this
old mouse protocol. - This sort of keypress-simulation is exactly why
we created the CAP_SYS_ADMIN restriction for TIOCL_SETSEL in the first
place.

For background on this mouse tracking mechanism, I had to read up on
it myself, but found the following two links helpful:
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Normal-tracking-mode
https://www.ibm.com/docs/en/aix/7.2?topic=x-xterm-command#xterm__mouse

(Remark on the side, the GPM client library normally gets its mouse
coordinates through a Unix Domain socket from the GPM daemon. It has
support for this xterm emulation mode as well, but it is only enabled
if $TERM starts with "xterm".)

In summary:

If it is not absolutely needed, I think it would be better to not
permit access to TIOCL_SELMOUSEREPORT after all.  It does not let
attackers simulate keypresses quite as easily as the other features,
but it does let them send such input with more limited control, and it
seems like an unnecessary risk if the feature is not needed by
anything but mouse daemons running as root, such as GPM and
Consolation.

Does this seem reasonable?  (Hanno, do you agree with this
assessment?)  I am by no means an expert in this terminal-mouse
interaction, I am happy to stand corrected if I am wrong here.

–Günther

On 2025-02-20 16:10, Günther Noack wrote:
> 
> Jared, can you please confirm whether Emacs works now with this patch
> in the kernel?
> 
> I am asking this because I realized that the patch had a bug.  We are
> erring in the "secure" direction, but not all TIOCL_SELMOUSEREPORT
> invocations work without CAP_SYS_ADMIN.

I confirmed that Emacs worked fine with 6.14-rc1.  My understanding is 
that the Emacs process relies only on TIOCL_SELPOINTER which it needs to 
do to draw the mouse pointer after Emacs' redisplay.  It's fine for 
TIOCL_SELMOUSEREPORT to not work in an unpriviliged Emacs.

> If this specific selection mode is not needed by Emacs, I think *the
> best thing would be to keep it guarded by CAP_SYS_ADMIN, after all*.

This sounds good to me.

Reading over a documentation proposal for TIOCL_SELMOUSEREPORT 
(https://lkml.org/lkml/2020/7/6/249), I can not imagine how a userspace 
program that was not acting as the mouse daemon could successfully use 
SELMOUSEREPORT as the mouse daemon will be fighting with it.  Any 
legitimate setting of mouse state (for example, setting the mouse x/y 
coordinate) would need to be done with the mouse daemon in the loop, in 
which case the mouse daemon might as well send the message itself.

   -- MJF

On Sat, Feb 08, 2025 at 07:18:22AM -0800, Jared Finder wrote:
> Hi, I'm the original reporter of this regression (noticed because it
> impacted GNU Emacs) and I'm wondering if there's any traction on creating an
> updated patch? This thread appears to have stalled out. I haven't seen any
> reply for three weeks.

It's already in 6.14-rc1 as commit 2f83e38a095f ("tty: Permit some
TIOCL_SETSEL modes without CAP_SYS_ADMIN").

thanks,

greg k-h

On 2025-02-08 07:28, Greg KH wrote:
> On Sat, Feb 08, 2025 at 07:18:22AM -0800, Jared Finder wrote:
>> Hi, I'm the original reporter of this regression (noticed because it
>> impacted GNU Emacs) and I'm wondering if there's any traction on 
>> creating an
>> updated patch? This thread appears to have stalled out. I haven't seen 
>> any
>> reply for three weeks.
> 
> It's already in 6.14-rc1 as commit 2f83e38a095f ("tty: Permit some
> TIOCL_SETSEL modes without CAP_SYS_ADMIN").

Great! Is this expected to get backported to 6.7 through 6.13? I would 
like to note the expected resolution correctly in Emacs' bug tracker.

   -- MJF

On Sat, Feb 08, 2025 at 08:03:27AM -0800, Jared Finder wrote:
> On 2025-02-08 07:28, Greg KH wrote:
> > On Sat, Feb 08, 2025 at 07:18:22AM -0800, Jared Finder wrote:
> > > Hi, I'm the original reporter of this regression (noticed because it
> > > impacted GNU Emacs) and I'm wondering if there's any traction on
> > > creating an
> > > updated patch? This thread appears to have stalled out. I haven't
> > > seen any
> > > reply for three weeks.
> > 
> > It's already in 6.14-rc1 as commit 2f83e38a095f ("tty: Permit some
> > TIOCL_SETSEL modes without CAP_SYS_ADMIN").
> 
> Great! Is this expected to get backported to 6.7 through 6.13? I would like
> to note the expected resolution correctly in Emacs' bug tracker.

Yes, it should show up in the next round of stable kernel updates later
this week.

But note, it will only show up in the 6.12.y and 6.13.y kernels, as all
others in your range are end-of-life and shouldn't be used by anyone at
this point in time.

thanks,

greg k-h