packages/systemd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!136 fix broken symbolic links when same link for different devices

Browse Source 
From: @yangmingtaip
Reviewed-by: @openeuler-basic
Signed-off-by: @openeuler-basic
This commit is contained in:
openeuler-ci-bot 2021-08-03 10:35:12 +00:00 committed by Gitee
commit 55493382eb
4 changed files with 271 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
backport-basic-stat-util-make-mtime-check-stricter-and-use-en.patch Normal file
View File

@ -0,0 +1,34 @@
From a59b0a9f768f6e27b25f4f1bab6de08842e78d74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Sekleta=CC=81r?= <msekleta@redhat.com>
Date: Thu, 5 Nov 2020 17:55:25 +0100
Subject: [PATCH] basic/stat-util: make mtime check stricter and use entire
timestamp
Note that st_mtime member of struct stat is defined as follows,
#define st_mtime st_mtim.tv_sec
Hence we omitted checking nanosecond part of the timestamp (struct
timespec) and possibly would miss modifications that happened within the
same second.
---
src/basic/stat-util.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c
index 4595ba7..b44d75c 100644
--- a/src/basic/stat-util.c
+++ b/src/basic/stat-util.c
@@ -394,7 +394,8 @@ bool stat_inode_unmodified(const struct stat *a, const struct stat *b) {
return a && b &&
(a->st_mode & S_IFMT) != 0 && /* We use the check for .st_mode if the structure was ever initialized */
((a->st_mode ^ b->st_mode) & S_IFMT) == 0 && /* same inode type */
- a->st_mtime == b->st_mtime &&
+ a->st_mtim.tv_sec == b->st_mtim.tv_sec &&
+ a->st_mtim.tv_nsec == b->st_mtim.tv_nsec &&
(!S_ISREG(a->st_mode) || a->st_size == b->st_size) && /* if regular file, compare file size */
a->st_dev == b->st_dev &&
a->st_ino == b->st_ino &&
--
2.23.0

53
backport-stat-util-add-stat_inode_unmodified-helper-that-chec.patch Normal file
View File

@ -0,0 +1,53 @@
From fee5c52ac260d021466c1062499f0ebd5241db5f Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 28 Apr 2020 18:16:25 +0200
Subject: [PATCH] stat-util: add stat_inode_unmodified() helper that checks if
an inode was modified
---
src/basic/stat-util.c | 21 +++++++++++++++++++++
src/basic/stat-util.h | 2 ++
2 files changed, 23 insertions(+)
diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c
index 2cd722c..4595ba7 100644
--- a/src/basic/stat-util.c
+++ b/src/basic/stat-util.c
@@ -379,3 +379,24 @@ int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret
return 0;
}
+
+bool stat_inode_unmodified(const struct stat *a, const struct stat *b) {
+
+ /* Returns if the specified stat structures reference the same, unmodified inode. This check tries to
+ * be reasonably careful when detecting changes: we check both inode and mtime, to cater for file
+ * systems where mtimes are fixed to 0 (think: ostree/nixos type installations). We also check file
+ * size, backing device, inode type and if this refers to a device not the major/minor.
+ *
+ * Note that we don't care if file attributes such as ownership or access mode change, this here is
+ * about contents of the file. The purpose here is to detect file contents changes, and nothing
+ * else. */
+
+ return a && b &&
+ (a->st_mode & S_IFMT) != 0 && /* We use the check for .st_mode if the structure was ever initialized */
+ ((a->st_mode ^ b->st_mode) & S_IFMT) == 0 && /* same inode type */
+ a->st_mtime == b->st_mtime &&
+ (!S_ISREG(a->st_mode) || a->st_size == b->st_size) && /* if regular file, compare file size */
+ a->st_dev == b->st_dev &&
+ a->st_ino == b->st_ino &&
+ (!(S_ISCHR(a->st_mode) || S_ISBLK(a->st_mode)) || a->st_rdev == b->st_rdev); /* if device node, also compare major/minor, because we can */
+}
diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h
index 7824af3..3665059 100644
--- a/src/basic/stat-util.h
+++ b/src/basic/stat-util.h
@@ -87,3 +87,5 @@ int fd_verify_directory(int fd);
int device_path_make_major_minor(mode_t mode, dev_t devno, char **ret);
int device_path_make_canonical(mode_t mode, dev_t devno, char **ret);
int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret_devno);
+
+bool stat_inode_unmodified(const struct stat *a, const struct stat *b);
--
2.23.0

174
backport-udev-make-algorithm-that-selects-highest-priority-de.patch Normal file
View File

@ -0,0 +1,174 @@
From 30f6dce62cb3a738b20253f2192270607c31b55b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Sekleta=CC=81r?= <msekleta@redhat.com>
Date: Fri, 23 Oct 2020 16:30:23 +0200
Subject: [PATCH] udev: make algorithm that selects highest priority devlink
less susceptible to race conditions
Previously it was very likely, when multiple contenders for the symlink
appear in parallel, that algorithm would select wrong symlink (i.e. one
with lower-priority).
Now the algorithm is much more defensive and when we detect change in
set of contenders for the symlink we reevaluate the selection. Same
happens when new symlink replaces already existing symlink that points
to different device node.
---
src/udev/udev-event.c | 7 ++++
src/udev/udev-node.c | 75 ++++++++++++++++++++++++++++++++++---------
2 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index ae78aef..1a8becb 100644
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -1017,6 +1017,13 @@ int udev_event_execute_rules(UdevEvent *event,
if (r < 0)
return log_device_debug_errno(dev, r, "Failed to update database under /run/udev/data/: %m");
+ /* Yes, we run update_devnode() twice, because in the first invocation, that is before update of udev database,
+ * it could happen that two contenders are replacing each other's symlink. Hence we run it again to make sure
+ * symlinks point to devices that claim them with the highest priority. */
+ r = update_devnode(event);
+ if (r < 0)
+ return r;
+
device_set_is_initialized(dev);
event->dev_db_clone = sd_device_unref(event->dev_db_clone);
diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c
index ce95e20..2694185 100644
--- a/src/udev/udev-node.c
+++ b/src/udev/udev-node.c
@@ -22,12 +22,15 @@
#include "path-util.h"
#include "selinux-util.h"
#include "smack-util.h"
+#include "stat-util.h"
#include "stdio-util.h"
#include "string-util.h"
#include "strxcpyx.h"
#include "udev-node.h"
#include "user-util.h"
+#define LINK_UPDATE_MAX_RETRIES 128
+
static int node_symlink(sd_device *dev, const char *node, const char *slink) {
_cleanup_free_ char *slink_dirname = NULL, *target = NULL;
const char *id_filename, *slink_tmp;
@@ -101,7 +104,9 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) {
if (rename(slink_tmp, slink) < 0) {
r = log_device_error_errno(dev, errno, "Failed to rename '%s' to '%s': %m", slink_tmp, slink);
(void) unlink(slink_tmp);
- }
+ } else
+ /* Tell caller that we replaced already existing symlink. */
+ r = 1;
return r;
}
@@ -194,7 +199,7 @@ static int link_update(sd_device *dev, const char *slink, bool add) {
_cleanup_free_ char *target = NULL, *filename = NULL, *dirname = NULL;
char name_enc[PATH_MAX];
const char *id_filename;
- int r;
+ int i, r, retries;
assert(dev);
assert(slink);
@@ -214,14 +219,6 @@ static int link_update(sd_device *dev, const char *slink, bool add) {
if (!add && unlink(filename) == 0)
(void) rmdir(dirname);
- r = link_find_prioritized(dev, add, dirname, &target);
- if (r < 0) {
- log_device_debug(dev, "No reference left, removing '%s'", slink);
- if (unlink(slink) == 0)
- (void) rmdir_parents(slink, "/");
- } else
- (void) node_symlink(dev, target, slink);
-
if (add)
do {
_cleanup_close_ int fd = -1;
@@ -234,7 +231,49 @@ static int link_update(sd_device *dev, const char *slink, bool add) {
r = -errno;
} while (r == -ENOENT);
- return r;
+ /* If the database entry is not written yet we will just do one iteration and possibly wrong symlink
+ * will be fixed in the second invocation. */
+ retries = sd_device_get_is_initialized(dev) > 0 ? LINK_UPDATE_MAX_RETRIES : 1;
+
+ for (i = 0; i < retries; i++) {
+ struct stat st1 = {}, st2 = {};
+
+ r = stat(dirname, &st1);
+ if (r < 0 && errno != ENOENT)
+ return -errno;
+
+ r = link_find_prioritized(dev, add, dirname, &target);
+ if (r == -ENOENT) {
+ log_device_debug(dev, "No reference left, removing '%s'", slink);
+ if (unlink(slink) == 0)
+ (void) rmdir_parents(slink, "/");
+
+ break;
+ } else if (r < 0)
+ return log_device_error_errno(dev, r, "Failed to determine highest priority symlink: %m");
+
+ r = node_symlink(dev, target, slink);
+ if (r < 0) {
+ (void) unlink(filename);
+ break;
+ } else if (r == 1)
+ /* We have replaced already existing symlink, possibly there is some other device trying
+ * to claim the same symlink. Let's do one more iteration to give us a chance to fix
+ * the error if other device actually claims the symlink with higher priority. */
+ continue;
+
+ /* Skip the second stat() if the first failed, stat_inode_unmodified() would return false regardless. */
+ if ((st1.st_mode & S_IFMT) != 0) {
+ r = stat(dirname, &st2);
+ if (r < 0 && errno != ENOENT)
+ return -errno;
+
+ if (stat_inode_unmodified(&st1, &st2))
+ break;
+ }
+ }
+
+ return i < LINK_UPDATE_MAX_RETRIES ? 0 : -ELOOP;
}
int udev_node_update_old_links(sd_device *dev, sd_device *dev_old) {
@@ -442,8 +481,11 @@ int udev_node_add(sd_device *dev, bool apply,
(void) node_symlink(dev, devnode, filename);
/* create/update symlinks, add symlinks to name index */
- FOREACH_DEVICE_DEVLINK(dev, devlink)
- (void) link_update(dev, devlink, true);
+ FOREACH_DEVICE_DEVLINK(dev, devlink) {
+ r = link_update(dev, devlink, true);
+ if (r < 0)
+ log_device_info_errno(dev, r, "Failed to update device symlinks: %m");
+ }
return 0;
}
@@ -456,8 +498,11 @@ int udev_node_remove(sd_device *dev) {
assert(dev);
/* remove/update symlinks, remove symlinks from name index */
- FOREACH_DEVICE_DEVLINK(dev, devlink)
- (void) link_update(dev, devlink, false);
+ FOREACH_DEVICE_DEVLINK(dev, devlink) {
+ r = link_update(dev, devlink, false);
+ if (r < 0)
+ log_device_info_errno(dev, r, "Failed to update device symlinks: %m");
+ }
r = xsprintf_dev_num_path_from_sd_device(dev, &filename);
if (r < 0)
--
2.23.0

11
systemd.spec
View File

@ -16,7 +16,7 @@
Name: systemd
Url: https://www.freedesktop.org/wiki/Software/systemd
Version: 243
Release: 42
Release: 43
License: MIT and LGPLv2+ and GPLv2+
Summary: System and Service Manager
@ -133,6 +133,9 @@ Patch0085: backport-execute-Fix-migration-from-DynamicUser-yes-to-no.patch
Patch0086: 0086-fix-CVE-2021-33910.patch
Patch0087: backport-units-restore-RemainAfterExit-yes-in-systemd-vconsol.patch
Patch0088: backport-udevd-don-t-kill-worker-in-manager_kill_workers-when.patch
Patch0089: backport-stat-util-add-stat_inode_unmodified-helper-that-chec.patch
Patch0090: backport-basic-stat-util-make-mtime-check-stricter-and-use-en.patch
Patch0091: backport-udev-make-algorithm-that-selects-highest-priority-de.patch
#openEuler
Patch9002: 1509-fix-journal-file-descriptors-leak-problems.patch
@ -1519,6 +1522,12 @@ fi
%exclude /usr/share/man/man3/*
%changelog
* Tue Aug 03 2021 yangmingtai <yangmingtai@huawei.com> - 243-43
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:udevd: fix broken symbolic links when same link for different devices
* Mon Aug 02 2021 fangxiuning <fangxiuning@huawei.com> - 243-42
- Type:bugfix
- ID:NA