On Fri, Sep 6, 2013 at 10:12 AM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > Hi Dmitry > > On Sun, Sep 1, 2013 at 4:07 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: >> If we have multiple sessions on a system, we normally don't want >> background sessions to read input events. Otherwise, it could capture >> passwords and more entered by the user on the foreground session. This is >> a real world problem as the recent XMir development showed: >> http://mjg59.dreamwidth.org/27327.html >> >> We currently rely on sessions to release input devices when being >> deactivated. This relies on trust across sessions. But that's not given on >> usual systems. We therefore need a way to control which processes have >> access to input devices. >> >> With VTs the kernel simply routed them through the active /dev/ttyX. This >> is not possible with evdev devices, though. Moreover, we want to avoid >> routing input-devices through some dispatcher-daemon in userspace (which >> would add some latency). >> >> This patch introduces EVIOCREVOKE. If called on an evdev fd, this revokes >> device-access irrecoverably for that *single* open-file. Hence, once you >> call EVIOCREVOKE on any dup()ed fd, all fds for that open-file will be >> rather useless now (but still valid compared to close()!). This allows us >> to pass fds directly to session-processes from a trusted source. The >> source keeps a dup()ed fd and revokes access once the session-process is >> no longer active. >> Compared to the EVIOCMUTE proposal, we can avoid the CAP_SYS_ADMIN >> restriction now as there is no way to revive the fd again. Hence, a user >> is free to call EVIOCREVOKE themself to kill the fd. >> >> Additionally, this ioctl allows multi-layer access-control (again compared >> to EVIOCMUTE which was limited to one layer via CAP_SYS_ADMIN). A middle >> layer can simply request a new open-file from the layer above and pass it >> to the layer below. Now each layer can call EVIOCREVOKE on the fds to >> revoke access for all layers below, at the expense of one fd per layer. >> >> There's already ongoing experimental user-space work which demonstrates >> how it can be used: >> http://lists.freedesktop.org/archives/systemd-devel/2013-August/012897.html > > I've been running this logind-feature for some time on my machine now. > I haven't seen any problems. Could you let us know what your plans are > for this patch? > 3.12 would be very nice so we could continue pushing the user-space parts. > There are even existing programs (like weston-launch) which could > already make great use of that with just a <10 line patch. I think we have good use cases for this - with revoke we can give input fds to a non-trusted, non-root display-severs and standalone KMS+evdev applications and reliably revoke them when their session is deactivated (vt switched away typically). EVIOCREVOKE is usable in logind and standalone setuid helpers such as weston-launch,. Everything other priviledge operation a display server needs can be done in logind or weston-launch, this ioctl is the final piece that lets us run X without root priviledges. Reviewed-by: Kristian Høgsberg <krh@xxxxxxxxxxxxx> >> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> >> --- >> Changes from v2: >> - Handle client->revoked like evdev->exist to avoid special-casing "revoked" >> >> drivers/input/evdev.c | 33 +++++++++++++++++++++++++++++---- >> include/uapi/linux/input.h | 1 + >> 2 files changed, 30 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c >> index d2b34fb..cf6bdff 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; >> + bool revoked; >> unsigned int bufsize; >> struct input_event buffer[]; >> }; >> @@ -164,6 +165,9 @@ static void evdev_pass_values(struct evdev_client *client, >> struct input_event event; >> bool wakeup = false; >> >> + if (client->revoked) >> + return; >> + >> event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ? >> mono : real); >> >> @@ -429,7 +433,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, >> if (retval) >> return retval; >> >> - if (!evdev->exist) { >> + if (!evdev->exist || client->revoked) { >> retval = -ENODEV; >> goto out; >> } >> @@ -511,7 +515,7 @@ static ssize_t evdev_read(struct file *file, char __user *buffer, >> if (!(file->f_flags & O_NONBLOCK)) { >> error = wait_event_interruptible(evdev->wait, >> client->packet_head != client->tail || >> - !evdev->exist); >> + !evdev->exist || client->revoked); >> if (error) >> return error; >> } >> @@ -529,7 +533,11 @@ static unsigned int evdev_poll(struct file *file, poll_table *wait) >> >> poll_wait(file, &evdev->wait, wait); >> >> - mask = evdev->exist ? POLLOUT | POLLWRNORM : POLLHUP | POLLERR; >> + if (evdev->exist && !client->revoked) >> + mask = POLLOUT | POLLWRNORM; >> + else >> + mask = POLLHUP | POLLERR; >> + >> if (client->packet_head != client->tail) >> mask |= POLLIN | POLLRDNORM; >> >> @@ -795,6 +803,17 @@ static int evdev_handle_mt_request(struct input_dev *dev, >> return 0; >> } >> >> +static int evdev_revoke(struct evdev *evdev, struct evdev_client *client, >> + struct file *file) >> +{ >> + client->revoked = true; >> + evdev_ungrab(evdev, client); >> + input_flush_device(&evdev->handle, file); >> + wake_up_interruptible(&evdev->wait); >> + >> + return 0; >> +} >> + >> static long evdev_do_ioctl(struct file *file, unsigned int cmd, >> void __user *p, int compat_mode) >> { >> @@ -857,6 +876,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, >> else >> return evdev_ungrab(evdev, client); >> >> + case EVIOCREVOKE: >> + if (p) >> + return -EINVAL; >> + else >> + return evdev_revoke(evdev, client, file); >> + >> case EVIOCSCLOCKID: >> if (copy_from_user(&i, p, sizeof(unsigned int))) >> return -EFAULT; >> @@ -1002,7 +1027,7 @@ static long evdev_ioctl_handler(struct file *file, unsigned int cmd, >> if (retval) >> return retval; >> >> - if (!evdev->exist) { >> + if (!evdev->exist || client->revoked) { >> retval = -ENODEV; >> goto out; >> } >> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h >> index 2fb6fae..d61c61c 100644 >> --- a/include/uapi/linux/input.h >> +++ b/include/uapi/linux/input.h >> @@ -152,6 +152,7 @@ struct input_keymap_entry { >> #define EVIOCGEFFECTS _IOR('E', 0x84, int) /* Report number of effects playable at the same time */ >> >> #define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */ >> +#define EVIOCREVOKE _IOW('E', 0x91, int) /* Revoke device access */ >> >> #define EVIOCSCLOCKID _IOW('E', 0xa0, int) /* Set clockid to be used for timestamps */ >> >> -- >> 1.8.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
