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))
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
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
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
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-m... 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
This requirement was overeagerly loosened in commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN"), but as it turns out,
(1) the logic I implemented there was inconsistent (apologies!),
(2) TIOCL_SELMOUSEREPORT might actually be a small security risk after all, and
(3) TIOCL_SELMOUSEREPORT is only meant to be used by the mouse daemon (GPM or Consolation), which runs as CAP_SYS_ADMIN already.
In more detail:
1. The previous patch has inconsistent logic:
In commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN"), we checked for sel_mode == TIOCL_SELMOUSEREPORT, but overlooked that the lower four bits of this "mode" parameter were actually used as an additional way to pass an argument. So the patch did actually still require CAP_SYS_ADMIN, if any of the mouse button bits are set, but did not require it if none of the mouse buttons bits are set.
This logic is inconsistent and was not intentional. We should have the same policies for using TIOCL_SELMOUSEREPORT independent of the value of the "hidden" mouse button argument.
I sent a separate documentation patch to the man page list with more details on TIOCL_SELMOUSEREPORT: https://lore.kernel.org/all/20250223091342.35523-2-gnoack3000@gmail.com/
2. TIOCL_SELMOUSEREPORT is indeed a potential security risk which can let an attacker simulate "keyboard" input to command line applications on the same terminal, like TIOCSTI and some other TIOCLINUX "selection mode" IOCTLs.
By enabling mouse reporting on a terminal and then injecting mouse reports through TIOCL_SELMOUSEREPORT, an attacker can simulate mouse movements on the same terminal, similar to the TIOCSTI keystroke injection attacks that were previously possible with TIOCSTI and other TIOCL_SETSEL selection modes.
Many programs (including libreadline/bash) are then prone to misinterpret these mouse reports as normal keyboard input because they do not expect input in the X11 mouse protocol form. The attacker does not have complete control over the escape sequence, but they can at least control the values of two consecutive bytes in the binary mouse reporting escape sequence.
I went into more detail on that in the discussion at https://lore.kernel.org/all/20250221.0a947528d8f3@gnoack.org/
It is not equally trivial to simulate arbitrary keystrokes as it was with TIOCSTI (commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled")), but the general mechanism is there, and together with the small number of existing legit use cases (see below), it would be better to revert back to requiring CAP_SYS_ADMIN for TIOCL_SELMOUSEREPORT, as it was already the case before commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN").
3. TIOCL_SELMOUSEREPORT is only used by the mouse daemons (GPM or Consolation), and they are the only legit use case:
To quote console_codes(4):
The mouse tracking facility is intended to return xterm(1)-compatible mouse status reports. Because the console driver has no way to know the device or type of the mouse, these reports are returned in the console input stream only when the virtual terminal driver receives a mouse update ioctl. These ioctls must be generated by a mouse-aware user-mode application such as the gpm(8) daemon.
Jared Finder has also confirmed in https://lore.kernel.org/all/491f3df9de6593df8e70dbe77614b026@finder.org/ that Emacs does not call TIOCL_SELMOUSEREPORT directly, and it would be difficult to find good reasons for doing that, given that it would interfere with the reports that GPM is sending.
More information on the interaction between GPM, terminals and the kernel with additional pointers is also available in this patch: https://lore.kernel.org/all/a773e48920aa104a65073671effbdee665c105fc.1603963...
For background on who else uses TIOCL_SELMOUSEREPORT: Debian Code search finds one page of results, the only two known callers are the two mouse daemons GPM and Consolation. (GPM does not show up in the search results because it uses literal numbers to refer to TIOCLINUX-related enums. I looked through GPM by hand instead. TIOCL_SELMOUSEREPORT is also not used from libgpm.) https://codesearch.debian.net/search?q=TIOCL_SELMOUSEREPORT
Cc: Jared Finder jared@finder.org Cc: Jann Horn jannh@google.com Cc: Hanno Böck hanno@hboeck.de Cc: Jiri Slaby jirislaby@kernel.org Cc: Kees Cook kees@kernel.org Cc: stable@vger.kernel.org Fixes: 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN") Signed-off-by: Günther Noack gnoack3000@gmail.com --- drivers/tty/vt/selection.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 0bd6544e30a6b..791e2f1f7c0b6 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -193,13 +193,12 @@ int set_selection_user(const struct tiocl_selection __user *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. + * TIOCL_SELCLEAR and TIOCL_SELPOINTER 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))
base-commit: 27102b38b8ca7ffb1622f27bcb41475d121fb67f
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:
- 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.
- 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
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Greg Kroah-Hartman
-
Günther Noack -
Günther Noack -
Jared Finder -
Kees Cook