Re: GPM & Emacs broken in Linux 6.7 -- ok to relax check?

[Günther Noack]

Hello!

On Fri, Nov 29, 2024 at 08:50:38PM +0100, Jann Horn wrote:
> +regression list, LKML, maintainers, patch authors
> 
> On Fri, Nov 29, 2024 at 8:38 PM Jared Finder <jared@xxxxxxxxxx> wrote:
> > The change to restrict access to TIOCLINUX that was added in Linux 6.7
> > breaks Emacs rendering of the mouse pointer. This change was previous
> > discussed in
> > https://lwn.net/ml/kernel-hardening/20230402160815.74760f87.hanno@xxxxxxxxx/.
> 
> This landed as commit 8d1b43f6a6df ("tty: Restrict access to
> TIOCLINUX' copy-and-paste subcommands").
> 
> #regzbot introduced: 8d1b43f6a6df

Thank you for reporting the bug, and thanks for forwarding, Jann!

> > An associated Emacs bug report, bug #74220, is discussed at
> > https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-11/msg00275.html.
> >
> > I wanted to ask if it made sense for the restriction to not apply to the
> > following three selection modes for TIOCL_SETSEL:
> >
> > TIOCL_SELPOINTER   3 /* show the pointer */
> > TIOCL_SELCLEAR   4 /* clear visibility of selection */
> > TIOCL_SELMOUSEREPORT   16 /* report beginning of selection */
> >
> > On a glance over the selection code, none of these interact with
> > vc_sel.buffer and therefore are unrelated to the exploit linked in the
> > original report. Only SELPOINTER is necessary to be available to fix
> > Emacs bug #74220. I imagine such a change would involve moving the
> > capability check from tioclinux(), case TIOCL_SETSEL to inside
> > vc_do_selection().

We did indeed miss that Emacs is using these IOCTLs directly.

To paraphrase what is happening, so that we are on the same page:

* Emacs includes the GPM header gpm.h and calls Gpm_DrawPointer(x, y, fd).
* Gpm_DrawPointer is implemented as a macro, which hardcodes all IOCTL constants
  (as it is the case in the entire GPM codebase), and it invokes ioctl(2)
  directly from the macro.

The Gpm_DrawPointer also gets called from other packages, including mc (Midnight
Commander), elinks and libt3widget (which supports the Tilde text editor).

https://codesearch.debian.net/search?q=Gpm_DrawPointer&literal=1&perpkg=1

* Midnight Commander and the Tilde text editor display the mouse cursor fine,
  also as a regular user.

* Elinks does not display the mouse cursor at all, independent of whether it is
  being run as root or not.  (Which makes me suspect that it might be an
  independent bug.)

* Emacs does not display the mouse cursor when run as normal user,
  but it works as root.

I can see how the three selection modes TIOCL_SELPOINTER, TIOCL_SELCLEAR and
TIOCL_SELMOUSEREPORT are related - TIOCL_SELPOINTER is the one that we need, but
TIOCL_SELPOINTER and TIOCL_SELCLEAR are the other two which can happen leading
up to that at the top of vc_selection()?

I also believe that what we *actually* want to guard here is the change to
vc_sel, so to propose a (somewhat simple-minded) patch, I guess we could put the
CAP_SYS_ADMIN check in vc_do_selection() right before the place where
vc_sel.start and vc_sel.end are being assigned?

Hanno, you are the original author of this patch and you have done a more
detailed analysis on the TIOCLINUX problems than me -- do you agree that this
weakened check would still be sufficient to protect against the TIOCLINUX
problems?  (Or in other words, if we permitted TIOCL_SELPOINTER, TIOCL_SELCLEAR
and TIOCL_SELMOUSEREPORT for non-CAP_SYS_ADMIN processes, would you still see a
way to misuse that functionality?)

Thanks,
—Günther