On Sun, Feb 23, 2025 at 09:54:50PM +0100, Günther Noack wrote:
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.
Greg and Jared: Friendly ping on this patch.
Could you please have a look if you can find the time?
To maybe explain in an overview again why this is safe:
The TIOCLINUX ioctl has various subcommands (uapi/linux/tiocl.h), and one of these has in turn more subcommands. The structure is:
* TIOCLINUX, with "subcodes": * TIOCL_SETSEL, with "selection modes": * TIOCL_SELCHAR * TIOCL_SELWORD * TIOCL_SELLINE * TIOCL_SELPOINTER * TIOCL_SELCLEAR * TIOCL_SELMOUSEREPORT * TIOCL_PASTESEL * TIOCL_SELLOADLUT * ...
While securing TIOCLINUX, we restricted access to various subcommands with CAP_SYS_ADMIN, but permitted different subcommands.
This table gives an overview of which TIOCL_SETSEL subcommands required CAP_SYS_ADMIN at which point in time:
point in time TIOCL_SETSEL sel_mode | 0 | 1 | 2 | 3 ----------------------|---|---|---|--- TIOCL_SELCHAR | | x | x | x TIOCL_SELWORD | | x | x | x TIOCL_SELLINE | | x | x | x TIOCL_SELPOINTER | | x | | TIOCL_SELCLEAR | | x | | TIOCL_SELMOUSEREPORT | | x | ? | x <-- This is the change
"x" means "requires CAP_SYS_ADMIN" "?" means "inconsistently requires CAP_SYS_ADMIN"
The points in time are:
(0) before we required CAP_SYS_ADMIN on TIOCLINUX subcommands (1) after commit 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands") (2) after commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN ") (3) after this patch ("tty: Require CAP_SYS_ADMIN for all usages of TIOCL_SELMOUSEREPORT")
This patch **reverts the behaviour for TIOCL_SELMOUSEREPORT back to what it was in phase (1)** after commit 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands"). We have double checked this in Emacs and GPM's source code earlier in this mail thread [1] and have confidence that this is better, because:
(a) TIOCL_SELMOUSEREPORT can maybe be abused after all, (b) it is not required for Emacs as we thought in patch (2) (c) the behavior I implemented in patch (2) was accidentally inconsistent
Again, apologies for the pointless back-and-forth on this fix, but it will be better after this iteration. I hope that this summary helps in the review. Please let me know if you have further questions.
Thanks, –Günther
[1] https://lore.kernel.org/all/491f3df9de6593df8e70dbe77614b026@finder.org/