Hi Kristian
On Tue, Apr 16, 2013 at 3:58 PM, Kristian Høgsberg <krh@xxxxxxxxxxxxx> wrote:
> This commit adds a new ioctl to the evdev device: EVIOCMUTE. The
> purpose of this ioctl it to temporarily block event delivery to a
> specific evdev client and prevent access to most of the ioctls.
>
> The use case is a setuid helper process for a display server, which
> opens input devices and passes the fds to the display server. On VT
> switch away from the server, it should stop reading events from its
> evdev input devices. However, if the display server runs as the user
> it can be ptraced or if the server loads user defined modules, the
> display server can no longer be trusted to not snoop on the evdev
> devices.
>
> The mute ioctl allows the setuid helper to mute evdev devices when
> switching away from the VT the server is running on. The idea is that
> the helper listens for the VT switching signals and when VT switch happens
> it notifies the display server, waits for the server to clean up and then
> the helper drops drm master (which also requires root), mutes evdev
> devices and the acks the VT switch.
>
> Why don't we just use revoke? The revoke syscall (when it's done) will
> work on filenames and shut down all fds open to the device. This will
> break all other processes that listen on evdev devices for legitimate
> reasons. It's the same reason we don't use the EVIOCGRAB ioctl for input
> devices. EVIOCMUTE lets the helper mute just the fd it gave to the
> display server without affecting anything else potentially using the device.
Other use-cases include SAK, forced VT switches and
system-compositors. I'd really like seeing this feature.
> Signed-off-by: Kristian Høgsberg <krh@xxxxxxxxxxxxx>
> ---
> drivers/input/evdev.c | 37 ++++++++++++++++++++++++++++++++++---
> include/uapi/linux/input.h | 1 +
> 2 files changed, 35 insertions(+), 3 deletions(-)
>
> This idea comes from work on Wayland and Weston [1], but the setuid helper
> should work and is required for a non-root X server to function correctly
> as well (ie, do proper vt switching).
>
> Kristian
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index f0f8928..cea6c35 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -48,6 +48,7 @@ struct evdev_client {
> struct evdev *evdev;
> struct list_head node;
> int clkid;
> + int muted;
Use "bool" instead.
> unsigned int bufsize;
> struct input_event buffer[];
> };
> @@ -130,8 +131,9 @@ static void evdev_events(struct input_handle *handle,
> evdev_pass_values(client, vals, count, time_mono, time_real);
> else
> list_for_each_entry_rcu(client, &evdev->client_list, node)
> - evdev_pass_values(client, vals, count,
> - time_mono, time_real);
> + if (!client->muted)
> + evdev_pass_values(client, vals, count,
> + time_mono, time_real);
Personally, I'd do this in evdev_pass_values(), but that's a matter of
taste.. And this would also allow muting grabbed devices. Because
currently input-grabs overwrite the mute flag which I think is odd. At
least mention this in the commit message so people know how it works
regarding input grabs.
>
> rcu_read_unlock();
> }
> @@ -674,6 +676,20 @@ static int evdev_handle_mt_request(struct input_dev *dev,
> return 0;
> }
>
> +static int evdev_mute(struct evdev *evdev, struct evdev_client *client)
> +{
> + client->muted = 1;
> +
> + return 0;
> +}
> +
> +static int evdev_unmute(struct evdev *evdev, struct evdev_client *client)
> +{
> + client->muted = 0;
> +
> + return 0;
> +}
> +
As I guess we want this feature to be atomic, we definitely need a
lock here. We also want, after EVIOCMUTE returns, that no other ioctl
will be allowed. Hence, we need a mutex that protects
evdev_do_ioctl(). A spinlock around "muted" won't help..
As we currently have no suitable lock in evdev_client I guess we need
to add one. In this case you can also remove these helpers and do:
client->muted = !!p;
in evdev_do_ioctl().
Note that a reader-writer lock would allow parallel ioctl execution if
only the EVIOCMUTE command takes the write-lock and the other ioctls
take a read lock.
Regards
David
> static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> void __user *p, int compat_mode)
> {
> @@ -687,12 +703,27 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> unsigned int size;
> int error;
>
> - /* First we check for fixed-length commands */
> + /* Handle ioctls allowed in muted mode first */
> switch (cmd) {
> + case EVIOCMUTE:
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + if (p)
> + return evdev_mute(evdev, client);
> + else
> + return evdev_unmute(evdev, client);
>
> case EVIOCGVERSION:
> return put_user(EV_VERSION, ip);
>
> + default:
> + if (client->muted)
> + return -EACCES;
> + }
> +
> + /* Now check for fixed-length commands */
> + switch (cmd) {
> case EVIOCGID:
> if (copy_to_user(p, &dev->id, sizeof(struct input_id)))
> return -EFAULT;
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 935119c..75eda72 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -154,6 +154,7 @@ struct input_keymap_entry {
> #define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
>
> #define EVIOCSCLOCKID _IOW('E', 0xa0, int) /* Set clockid to be used for timestamps */
> +#define EVIOCMUTE _IOW('E', 0xb0, int) /* Mute event delivery */
>
> /*
> * Device properties and quirks
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
