Merge pull request #17190 from poettering/udev-opath

udev: manipulate device nodes with O_PATH

man/tmpfiles.d.xml

@@ -92,7 +92,7 @@ A+    /path-or-glob/to/append/acls/recursively -    -    -     -           POSIX
     directories during boot and to do periodic cleanup afterwards. See
     <citerefentry><refentrytitle>systemd-tmpfiles</refentrytitle><manvolnum>5</manvolnum></citerefentry> for
     the description of <filename>systemd-tmpfiles-setup.service</filename>,
-    <filename>systemd-tmpfiles-cleanup.service</filename>, and associated units.</para>
+    <filename>systemd-tmpfiles-clean.service</filename>, and associated units.</para>
 
     <para>System daemons frequently require private runtime directories below <filename>/run</filename> to
     store communication sockets and similar. For these, it is better to use

src/basic/fs-util.c

@@ -312,6 +312,25 @@ int fchmod_opath(int fd, mode_t m) {
         return 0;
 }
 
+int futimens_opath(int fd, const struct timespec ts[2]) {
+        char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
+
+        /* Similar to fchmod_path() but for futimens() */
+
+        xsprintf(procfs_path, "/proc/self/fd/%i", fd);
+        if (utimensat(AT_FDCWD, procfs_path, ts, 0) < 0) {
+                if (errno != ENOENT)
+                        return -errno;
+
+                if (proc_mounted() == 0)
+                        return -ENOSYS; /* if we have no /proc/, the concept is not implementable */
+
+                return -ENOENT;
+        }
+
+        return 0;
+}
+
 int stat_warn_permissions(const char *path, const struct stat *st) {
         assert(path);
         assert(st);

src/basic/fs-util.h

@@ -38,6 +38,8 @@ int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid);
 int fchmod_umask(int fd, mode_t mode);
 int fchmod_opath(int fd, mode_t m);
 
+int futimens_opath(int fd, const struct timespec ts[2]);
+
 int fd_warn_permissions(const char *path, int fd);
 int stat_warn_permissions(const char *path, const struct stat *st);
 

src/basic/selinux-util.c

@@ -205,14 +205,11 @@ static int mac_selinux_reload(int seqno) {
 
 int mac_selinux_fix_container(const char *path, const char *inside_path, LabelFixFlags flags) {
 
-#if HAVE_SELINUX
-        char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
-        _cleanup_freecon_ char* fcon = NULL;
-        _cleanup_close_ int fd = -1;
-        struct stat st;
-        int r;
-
         assert(path);
+        assert(inside_path);
+
+#if HAVE_SELINUX
+        _cleanup_close_ int fd = -1;
 
         /* if mac_selinux_init() wasn't called before we are a NOOP */
         if (!label_hnd)
@@ -227,6 +224,27 @@ int mac_selinux_fix_container(const char *path, const char *inside_path, LabelFi
                 return -errno;
         }
 
+        return mac_selinux_fix_container_fd(fd, path, inside_path, flags);
+#endif
+
+        return 0;
+}
+
+int mac_selinux_fix_container_fd(int fd, const char *path, const char *inside_path, LabelFixFlags flags) {
+
+        assert(fd >= 0);
+        assert(inside_path);
+
+#if HAVE_SELINUX
+        char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
+        _cleanup_freecon_ char* fcon = NULL;
+        struct stat st;
+        int r;
+
+        /* if mac_selinux_init() wasn't called before we are a NOOP */
+        if (!label_hnd)
+                return 0;
+
         if (fstat(fd, &st) < 0)
                 return -errno;
 
@@ -234,12 +252,11 @@ int mac_selinux_fix_container(const char *path, const char *inside_path, LabelFi
         mac_selinux_maybe_reload();
 
         if (selabel_lookup_raw(label_hnd, &fcon, inside_path, st.st_mode) < 0) {
-                r = -errno;
-
                 /* If there's no label to set, then exit without warning */
-                if (r == -ENOENT)
+                if (errno == ENOENT)
                         return 0;
 
+                r = -errno;
                 goto fail;
         }
 
@@ -247,16 +264,16 @@ int mac_selinux_fix_container(const char *path, const char *inside_path, LabelFi
         if (setfilecon_raw(procfs_path, fcon) < 0) {
                 _cleanup_freecon_ char *oldcon = NULL;
 
-                r = -errno;
-
                 /* If the FS doesn't support labels, then exit without warning */
-                if (r == -EOPNOTSUPP)
+                if (ERRNO_IS_NOT_SUPPORTED(errno))
                         return 0;
 
                 /* It the FS is read-only and we were told to ignore failures caused by that, suppress error */
-                if (r == -EROFS && (flags & LABEL_IGNORE_EROFS))
+                if (errno == EROFS && (flags & LABEL_IGNORE_EROFS))
                         return 0;
 
+                r = -errno;
+
                 /* If the old label is identical to the new one, suppress any kind of error */
                 if (getfilecon_raw(procfs_path, &oldcon) >= 0 && streq(fcon, oldcon))
                         return 0;
@@ -267,7 +284,7 @@ int mac_selinux_fix_container(const char *path, const char *inside_path, LabelFi
         return 0;
 
 fail:
-        return log_enforcing_errno(r, "Unable to fix SELinux security context of %s (%s): %m", path, inside_path);
+        return log_enforcing_errno(r, "Unable to fix SELinux security context of %s (%s): %m", strna(path), strna(inside_path));
 #endif
 
         return 0;
@@ -275,11 +292,12 @@ fail:
 
 int mac_selinux_apply(const char *path, const char *label) {
 
+        assert(path);
+
 #if HAVE_SELINUX
         if (!mac_selinux_use())
                 return 0;
 
-        assert(path);
         assert(label);
 
         if (setfilecon(path, label) < 0)
@@ -288,6 +306,22 @@ int mac_selinux_apply(const char *path, const char *label) {
         return 0;
 }
 
+int mac_selinux_apply_fd(int fd, const char *path, const char *label) {
+
+        assert(fd >= 0);
+
+#if HAVE_SELINUX
+        if (!mac_selinux_use())
+                return 0;
+
+        assert(label);
+
+        if (fsetfilecon(fd, label) < 0)
+                return log_enforcing_errno(errno, "Failed to set SELinux security context %s on path %s: %m", label, strna(path));
+#endif
+        return 0;
+}
+
 int mac_selinux_get_create_label_from_exe(const char *exe, char **label) {
 #if HAVE_SELINUX
         _cleanup_freecon_ char *mycon = NULL, *fcon = NULL;

src/basic/selinux-util.h

@@ -28,7 +28,13 @@ static inline int mac_selinux_fix(const char *path, LabelFixFlags flags) {
         return mac_selinux_fix_container(path, path, flags);
 }
 
+int mac_selinux_fix_container_fd(int fd, const char *path, const char *inside_path, LabelFixFlags flags);
+static inline int mac_selinux_fix_fd(int fd, const char *path, LabelFixFlags flags) {
+        return mac_selinux_fix_container_fd(fd, path, path, flags);
+}
+
 int mac_selinux_apply(const char *path, const char *label);
+int mac_selinux_apply_fd(int fd, const char *path, const char *label);
 
 int mac_selinux_get_create_label_from_exe(const char *exe, char **label);
 int mac_selinux_get_our_label(char **label);

src/shared/udev-util.c

@@ -288,10 +288,12 @@ int device_is_renaming(sd_device *dev) {
         assert(dev);
 
         r = sd_device_get_property_value(dev, "ID_RENAMING", NULL);
-        if (r < 0 && r != -ENOENT)
+        if (r == -ENOENT)
+                return false;
+        if (r < 0)
                 return r;
 
-        return r >= 0;
+        return true;
 }
 
 bool device_for_action(sd_device *dev, DeviceAction action) {

src/udev/udev-node.c

@@ -273,9 +273,10 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac,
                                   mode_t mode, uid_t uid, gid_t gid,
                                   OrderedHashmap *seclabel_list) {
         const char *devnode, *subsystem, *id_filename = NULL;
+        bool apply_mode, apply_uid, apply_gid;
+        _cleanup_close_ int node_fd = -1;
         struct stat stats;
         dev_t devnum;
-        bool apply_mode, apply_uid, apply_gid;
         int r;
 
         assert(dev);
@@ -296,16 +297,25 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac,
         else
                 mode |= S_IFCHR;
 
-        if (lstat(devnode, &stats) < 0) {
+        node_fd = open(devnode, O_PATH|O_NOFOLLOW|O_CLOEXEC);
-                if (errno == ENOENT)
+        if (node_fd < 0) {
-                        return 0; /* this is necessarily racey, so ignore missing the device */
+                if (errno == ENOENT) {
-                return log_device_debug_errno(dev, errno, "cannot stat() node %s: %m", devnode);
+                        log_device_debug_errno(dev, errno, "Device node %s is missing, skipping handling.", devnode);
+                        return 0; /* This is necessarily racey, so ignore missing the device */
                 }
 
-        if ((mode != MODE_INVALID && (stats.st_mode & S_IFMT) != (mode & S_IFMT)) || stats.st_rdev != devnum)
+                return log_device_debug_errno(dev, errno, "Cannot open node %s: %m", devnode);
-                return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST),
+        }
-                                              "Found node '%s' with non-matching devnum %s, skip handling",
+
+        if (fstat(node_fd, &stats) < 0)
+                return log_device_debug_errno(dev, errno, "cannot stat() node %s: %m", devnode);
+
+        if ((mode != MODE_INVALID && (stats.st_mode & S_IFMT) != (mode & S_IFMT)) || stats.st_rdev != devnum) {
+                log_device_debug(dev, "Found node '%s' with non-matching devnum %s, skipping handling.",
                                  devnode, id_filename);
+                return 0; /* We might process a device that already got replaced by the time we have a look
+                           * at it, handle this gracefully and step away. */
+        }
 
         apply_mode = mode != MODE_INVALID && (stats.st_mode & 0777) != (mode & 0777);
         apply_uid = uid_is_valid(uid) && stats.st_uid != uid;
@@ -322,7 +332,7 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac,
                                          gid_is_valid(gid) ? gid : stats.st_gid,
                                          mode != MODE_INVALID ? mode & 0777 : stats.st_mode & 0777);
 
-                        r = chmod_and_chown(devnode, mode, uid, gid);
+                        r = fchmod_and_chown(node_fd, mode, uid, gid);
                         if (r < 0)
                                 log_device_full_errno(dev, r == -ENOENT ? LOG_DEBUG : LOG_ERR, r,
                                                       "Failed to set owner/mode of %s to uid=" UID_FMT
@@ -345,7 +355,7 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac,
                         if (streq(name, "selinux")) {
                                 selinux = true;
 
-                                q = mac_selinux_apply(devnode, label);
+                                q = mac_selinux_apply_fd(node_fd, devnode, label);
                                 if (q < 0)
                                         log_device_full_errno(dev, q == -ENOENT ? LOG_DEBUG : LOG_ERR, q,
                                                               "SECLABEL: failed to set SELinux label '%s': %m", label);
@@ -355,7 +365,7 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac,
                         } else if (streq(name, "smack")) {
                                 smack = true;
 
-                                q = mac_smack_apply(devnode, SMACK_ATTR_ACCESS, label);
+                                q = mac_smack_apply_fd(node_fd, SMACK_ATTR_ACCESS, label);
                                 if (q < 0)
                                         log_device_full_errno(dev, q == -ENOENT ? LOG_DEBUG : LOG_ERR, q,
                                                               "SECLABEL: failed to set SMACK label '%s': %m", label);
@@ -368,13 +378,15 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac,
 
                 /* set the defaults */
                 if (!selinux)
-                        (void) mac_selinux_fix(devnode, LABEL_IGNORE_ENOENT);
+                        (void) mac_selinux_fix_fd(node_fd, devnode, LABEL_IGNORE_ENOENT);
                 if (!smack)
-                        (void) mac_smack_apply(devnode, SMACK_ATTR_ACCESS, NULL);
+                        (void) mac_smack_apply_fd(node_fd, SMACK_ATTR_ACCESS, NULL);
         }
 
         /* always update timestamp when we re-use the node, like on media change events */
-        (void) utimensat(AT_FDCWD, devnode, NULL, 0);
+        r = futimens_opath(node_fd, NULL);
+        if (r < 0)
+                log_device_debug_errno(dev, r, "Failed to adjust timestamp of node %s: %m", devnode);
 
         return r;
 }