From ab3a7372d21bd41ac6d4ec574bf2ed5c63ae9a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 29 Jun 2019 14:56:45 +0200 Subject: [PATCH 1/2] udev: tiny fix to debug message --- src/udev/udev-node.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 7d254785776..01cd2f408a0 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -297,7 +297,7 @@ static int node_permissions_apply(sd_device *dev, bool apply, mode |= S_IFCHR; if (lstat(devnode, &stats) < 0) - return log_device_debug_errno(dev, errno, "cannot stat() node '%s' (%m)", devnode); + return log_device_debug_errno(dev, errno, "cannot stat() node %s: %m", devnode); if (((stats.st_mode & S_IFMT) != (mode & S_IFMT)) || (stats.st_rdev != devnum)) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), "Found node '%s' with non-matching devnum %s, skip handling", From 3708c0f455931c8cb5fed149923e5a995c2e3fbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 29 Jun 2019 15:08:11 +0200 Subject: [PATCH 2/2] udev: don't force device ownership and mode on every event This partially reverts 25de7aa7b90c23d33ea50ada1e50c5834a414237. I don't think the change was intended there. The problem I'm trying to solve: for /dev/kvm we get first an ADD uevent, and then CHANGE whenever something connects or disconnects to the character device. The rules in 50-default-udev.rules set UID, GID, and MODE on ADD, but not on CHANGE. When the change event happens, we would reset the ownership and permissions. This happens because node_permissions_apply() would (after 25de7aa7b90c23d33) set uid=gid=0 if they weren't set by the rules. So let's only pass uid/gid/mode to node_permissions_apply() if appropriately configured. Also let node_permissions_apply() do the skip of uid/gid/mode if not set, and rename "always_apply" to more closely reflect its meaning. --- src/udev/udev-event.c | 28 ++++++++-------------------- src/udev/udev-node.c | 22 +++++++++++++++------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 025a37bc26b..dbdb9065dcd 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -819,7 +819,6 @@ static int rename_netif(UdevEvent *event) { static int update_devnode(UdevEvent *event) { sd_device *dev = event->dev; - bool apply; int r; r = sd_device_get_devnum(dev, NULL); @@ -834,17 +833,13 @@ static int update_devnode(UdevEvent *event) { if (!uid_is_valid(event->uid)) { r = device_get_devnode_uid(dev, &event->uid); - if (r == -ENOENT) - event->uid = 0; - else if (r < 0) + if (r < 0 && r != -ENOENT) return log_device_error_errno(dev, r, "Failed to get devnode UID: %m"); } if (!gid_is_valid(event->gid)) { r = device_get_devnode_gid(dev, &event->gid); - if (r == -ENOENT) - event->gid = 0; - else if (r < 0) + if (r < 0 && r != -ENOENT) return log_device_error_errno(dev, r, "Failed to get devnode GID: %m"); } @@ -852,21 +847,14 @@ static int update_devnode(UdevEvent *event) { r = device_get_devnode_mode(dev, &event->mode); if (r < 0 && r != -ENOENT) return log_device_error_errno(dev, r, "Failed to get devnode mode: %m"); - if (r == -ENOENT) { - if (event->gid > 0) - /* default 0660 if a group is assigned */ - event->mode = 0660; - else - /* default 0600 */ - event->mode = 0600; - } } + if (event->mode == MODE_INVALID && gid_is_valid(event->gid) && event->gid > 0) + /* If group is set, but mode is not set, "upgrade" mode for the group. */ + event->mode = 0660; - apply = device_for_action(dev, DEVICE_ACTION_ADD) || - uid_is_valid(event->uid) || - gid_is_valid(event->gid) || - event->mode != MODE_INVALID; - return udev_node_add(dev, apply, event->mode, event->uid, event->gid, event->seclabel_list); + bool apply_mac = device_for_action(dev, DEVICE_ACTION_ADD); + + return udev_node_add(dev, apply_mac, event->mode, event->uid, event->gid, event->seclabel_list); } static void event_execute_rules_on_remove( diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 01cd2f408a0..7e3447f7fa3 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -26,6 +26,7 @@ #include "string-util.h" #include "strxcpyx.h" #include "udev-node.h" +#include "user-util.h" static int node_symlink(sd_device *dev, const char *node, const char *slink) { _cleanup_free_ char *slink_dirname = NULL, *target = NULL; @@ -270,13 +271,14 @@ int udev_node_update_old_links(sd_device *dev, sd_device *dev_old) { return 0; } -static int node_permissions_apply(sd_device *dev, bool apply, +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; struct stat stats; dev_t devnum; - int r = 0; + bool apply_mode, apply_uid, apply_gid; + int r; assert(dev); @@ -299,21 +301,27 @@ static int node_permissions_apply(sd_device *dev, bool apply, if (lstat(devnode, &stats) < 0) return log_device_debug_errno(dev, errno, "cannot stat() node %s: %m", devnode); - if (((stats.st_mode & S_IFMT) != (mode & S_IFMT)) || (stats.st_rdev != devnum)) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), "Found node '%s' with non-matching devnum %s, skip handling", + if ((mode != MODE_INVALID && (stats.st_mode & S_IFMT) != (mode & S_IFMT)) || stats.st_rdev != devnum) + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), + "Found node '%s' with non-matching devnum %s, skip handling", devnode, id_filename); - if (apply) { + apply_mode = mode != MODE_INVALID && (stats.st_mode & 0777) != (mode & 0777); + apply_uid = uid_is_valid(uid) && stats.st_uid != uid; + apply_gid = gid_is_valid(gid) && stats.st_gid != gid; + + if (apply_mode || apply_uid || apply_gid || apply_mac) { bool selinux = false, smack = false; const char *name, *label; Iterator i; - if ((stats.st_mode & 0777) != (mode & 0777) || stats.st_uid != uid || stats.st_gid != gid) { + if (apply_mode || apply_uid || apply_gid) { log_device_debug(dev, "Setting permissions %s, %#o, uid=%u, gid=%u", devnode, mode, uid, gid); r = chmod_and_chown(devnode, mode, uid, gid); if (r < 0) - log_device_warning_errno(dev, r, "Failed to set owner/mode of %s to uid=" UID_FMT ", gid=" GID_FMT ", mode=%#o: %m", devnode, uid, gid, mode); + log_device_warning_errno(dev, r, "Failed to set owner/mode of %s to uid=" UID_FMT ", gid=" GID_FMT ", mode=%#o: %m", + devnode, uid, gid, mode); } else log_device_debug(dev, "Preserve permissions of %s, %#o, uid=%u, gid=%u", devnode, mode, uid, gid);