On Tue, 17.09.13 17:39, David Herrmann ([email protected]) wrote: > " <arg name=\"force\" type=\"b\"/>\n" \ > " </method>\n" \ > " <method name=\"DropControl\"/>\n" \ > + " <method name=\"RequestDevice\">\n" \ > + " <arg name=\"node\" type=\"s\" direction=\"in\"/>\n" \ > + " <arg name=\"fd\" type=\"h\" direction=\"out\"/>\n" \ > + " <arg name=\"paused\" type=\"b\" direction=\"out\"/>\n" \ > + " </method>\n" \ > + " <method name=\"ReleaseDevice\">\n" \ > + " <arg name=\"node\" type=\"s\"/>\n" \ > + " </method>\n" > \
Please rename this pair to TakeDevice() and ReleaseDevice(). > index 0000000..659f161 > --- /dev/null > +++ b/src/login/logind-session-device.c > @@ -0,0 +1,489 @@ > +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ > + > +/*** > + This file is part of systemd. > + > + Copyright 2013 David Herrmann > + > + systemd is free software; you can redistribute it and/or modify it > + under the terms of the GNU Lesser General Public License as published by > + the Free Software Foundation; either version 2.1 of the License, or > + (at your option) any later version. > + > + systemd is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public License > + along with systemd; If not, see <http://www.gnu.org/licenses/>. > +***/ > + > +#include <assert.h> > +#include <fcntl.h> > +#include <libudev.h> > +#include <linux/input.h> > +#include <linux/ioctl.h> > +#include <string.h> > +#include <sys/ioctl.h> > +#include <sys/stat.h> > +#include <sys/types.h> > +#include <unistd.h> > + > +#include "dbus-common.h" > +#include "logind-session-device.h" > +#include "util.h" > + > +enum SessionDeviceNotifications { > + SESSION_DEVICE_RESUME, > + SESSION_DEVICE_TRY_PAUSE, > + SESSION_DEVICE_PAUSE, > + SESSION_DEVICE_RELEASE, > +}; > + > +static void session_device_notify(SessionDevice *sd, enum > SessionDeviceNotifications type) { > + _cleanup_dbus_message_unref_ DBusMessage *m = NULL; > + _cleanup_free_ char *path = NULL; > + const char *t = NULL; > + > + assert(sd); > + > + if (!sd->session->controller) > + return; > + > + path = session_bus_path(sd->session); > + if (!path) > + return; > + > + m = dbus_message_new_signal(path, > + "org.freedesktop.login1.Session", > + (type == SESSION_DEVICE_RESUME) ? > "ResumeDevice" : "PauseDevice"); > + if (!m) > + return; > + > + if (!dbus_message_set_destination(m, sd->session->controller)) > + return; > + > + switch (type) { > + case SESSION_DEVICE_RESUME: > + if (!dbus_message_append_args(m, > + DBUS_TYPE_STRING, &sd->node, > + DBUS_TYPE_UNIX_FD, &sd->fd, > + DBUS_TYPE_INVALID)) > + return; > + break; > + case SESSION_DEVICE_TRY_PAUSE: > + t = "pause"; > + break; > + case SESSION_DEVICE_PAUSE: > + t = "force"; > + break; > + case SESSION_DEVICE_RELEASE: > + t = "gone"; > + break; > + default: > + return; > + } > + > + if (t && !dbus_message_append_args(m, > + DBUS_TYPE_STRING, &sd->node, > + DBUS_TYPE_STRING, &t, > + DBUS_TYPE_INVALID)) > + return; > + > + dbus_connection_send(sd->session->manager->bus, m, NULL); > +} > + > +static int sd_eviocrevoke(int fd) > +{ "{" please on the same line as the function name. > + static bool warned; > + int r; > + > +#ifndef EVIOCREVOKE > +# define EVIOCREVOKE _IOW('E', 0x91, int) > +#endif Please move this to missing.h. > + > + assert(fd >= 0); > + > + r = ioctl(fd, EVIOCREVOKE, 1); > + if (r < 0) { > + r = -errno; > + if (r == -ENOSYS && !warned) { > + warned = true; > + log_warning("kernel does not support > evdev-revocation"); > + } > + } > + > + return 0; > +} > + > +static int sd_drmsetmaster(int fd) > +{ "{"... > + int r; > + > +#ifndef DRM_IOCTL_SET_MASTER > +# define DRM_IOCTL_SET_MASTER _IO('d', 0x1e) > +#endif → missing.h! > + > + assert(fd >= 0); > + > + r = ioctl(fd, DRM_IOCTL_SET_MASTER, 0); > + if (r < 0) > + return -errno; > + > + return 0; > +} > + > +static int sd_drmdropmaster(int fd) > +{ "{".... > + int r; > + > +#ifndef DRM_IOCTL_DROP_MASTER > +# define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f) > +#endif → missing.h > + switch (sd->type) { > + case DEVICE_TYPE_DRM: > + if (active) { > + sd_drmsetmaster(fd); > + } else { Please remove "{}". > + dnum = udev_device_get_devnum(dev); > + sysname = udev_device_get_sysname(dev); > + type = DEVICE_TYPE_UNKNOWN; > + > + if (major(dnum) == 29) { > + if (startswith(sysname, "fb")) > + type = DEVICE_TYPE_FBDEV; > + } else if (major(dnum) == 226) { > + if (startswith(sysname, "card")) > + type = DEVICE_TYPE_DRM; > + } else if (major(dnum) == 13) { > + if (startswith(sysname, "event")) > + type = DEVICE_TYPE_EVDEV; Please use udev_device_get_subsystem() for this! Comparing majors is evil! > + dev = udev_device_new_from_devnum(sd->session->manager->udev, 'c', > st.st_rdev); > + if (!dev) > + return -ENODEV; > + sp = udev_device_get_syspath(dev); > + > + /* Only allow access to "uaccess" tagged devices! */ > + if (!udev_device_has_tag(dev, "uaccess")) { > + r = -EPERM; > + goto err_dev; > + } > + This sounds wrong, after all the suer could access the device directly anyway if uaccess is set. > + /* Only allow opening the _real_ device node. This is the node > provided > + * by devtmpfs! Issue a readlink() if you are not sure. We cannot > allow > + * any path here as user-space could otherwise trigger OOM by > requesting > + * duplicates. */ > + real_node = udev_device_get_devnode(dev); > + if (!streq_ptr(real_node, node)) { > + r = -EINVAL; > + goto err_dev; > + } I think it would make sense to drop this. Instead verify with path_startswith() that the specified path is in /dev. Then do an lstat() on the specified device path, and verify the backing major/minor matches the one of /dev, so that people cannot use /dev/shm to confuse us. > +typedef struct SessionDevice SessionDevice; > + > +#include "list.h" > +#include "util.h" > +#include "logind.h" > +#include "logind-device.h" > +#include "logind-seat.h" > +#include "logind-session.h" > + > +enum DeviceType { > + DEVICE_TYPE_UNKNOWN, > + DEVICE_TYPE_FBDEV, > + DEVICE_TYPE_DRM, > + DEVICE_TYPE_EVDEV, > +}; exported enums plese with a typdef! Lennart -- Lennart Poettering - Red Hat, Inc. _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
