Merge a050031d05 into b7eefa1996

History:
- Before 331aa7aa15, a symlink creation
  requested by a new event replaced an existing symlink with the same
  priority.
- With 331aa7aa15 (v254), if an existing symlink
  has the same priority with the newly requested one, then the new request
  was skipped, and the existing symlink was kept. But this caused #28141.
- With 7ec5ce5673 (v255, and backported to v254.6),
  the behavior is restored prior to the 331aa7aa15.
  But, it is pointed out that the replacing symlink may be dangerous.
  See discussion in https://github.com/systemd/systemd/pull/34482#issuecomment-2362115374.

This introduce .UDEV_REPLACE_SYMLINK_WITH_SAME_PRIORITY flag, and set the flag
for dm devices. Why? We need to consider several points:
- On boot time, devices appear in random order. So, if e.g. sda and sdb
  requests the same symlink, then there are no way to guarantee which
  device will own the symlink, as these devices will be processed by
  udevd in parallel. Since, there are no guarantee, we can change the
  logic which device will win to own the symlink, that is, the first
  wins (with this PR) or the last wins (the current behavior).
- After the system is booted, e.g. when a USB stick (sdb) is inserted and
  it requests a symlink which is currently owned by sda. In that case,
  the current behavior makes the new device (sdb) steal the ownership of
  the symlink. But, that may be problematic as a user or a program may
  assume that the symlink is persistent, and data may be lost or stolen.
  We can easily create such hackish USB stick by creating a partition
  which has the same partition UUID on the stick.
  With this commit, new devices cannot override the ownership of the symlink,
  so such the issue will not be triggered anymore.
- As you can see the change in the test code for integrity in this commit,
  this commit is also important for dm-integrity. Without this PR, the data
  partition and the DM devices both request the same partition UUID symlink,
  and the data device gained the ownership, as a synthetic event for the
  underlying data device is triggered after the DM device is activated,
  and it steels the ownership from the DM device. People may mistakenly
  mounts the underlying data device rather than the DM device, and may causes
  data loss or corruption. With this PR, the underlying device will not
  gain the symlink, and blkid command always returns the DM device.
- Unfortunately, this re-introduce issue #28468, which is triggered for
  spurious ISO images. But, it can be easily avoided by reapplying the
  'workaround' done by df1dccd255.

Fixes #31448.
Replaces #34482, #33572, #32981.

rules.d/60-persistent-storage.rules.in

@@ -136,6 +136,12 @@ KERNEL!="sr*|mmcblk[0-9]boot[0-9]", IMPORT{builtin}="blkid"
 LABEL="persistent_storage_blkid_probe_end"
 {% endif %}
 
+# Decrease devlink priority for whole disk of ISO hybrid images, and make the
+# priority for partitions in the image relatively higher. This is for the case
+# that a disk and one of its partition have the same label or so.
+# See issue #28468.
+ENV{ID_FS_TYPE}=="iso9660", ENV{DEVTYPE}=="disk", OPTIONS+="link_priority=-10"
+
 # by-label/by-uuid links (filesystem metadata)
 ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
 ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"

rules.d/99-systemd.rules.in

@@ -22,6 +22,10 @@ SUBSYSTEM=="ubi", TAG+="systemd"
 
 SUBSYSTEM=="block", TAG+="systemd"
 
+# Make dm devices can replace existing symlinks with the same priority.
+# Otherwise, activating a device while another is being deactivated may fail.
+SUBSYSTEM=="block", KERNEL=="dm-*", ENV{.UDEV_REPLACE_SYMLINK_WITH_SAME_PRIORITY}="1"
+
 # When a dm device is first created, it's just an empty container. Ignore it.
 # DM_NAME is not set in this case, but it's set on spurious "add" events that occur later.
 SUBSYSTEM=="block", ACTION=="add", KERNEL=="dm-*", ENV{DM_NAME}!="?*", ENV{SYSTEMD_READY}="0"

src/udev/udev-node.c

@@ -462,6 +462,18 @@ static int link_update(sd_device *dev, const char *slink, bool add) {
                                          * another device. Hence, it is not necessary to recreate it. */
                                         return 0;
 
+                                /* When the priorities are equivalent, replace symlink only when it is
+                                 * explicitly requested. This is necessary for DM devices, otherwise
+                                 * activating a device while another is being deactivated may fail. See issue
+                                 * #28141. However, in general, replacing symlink with a same-priority device
+                                 * is dangerous. For example, when managing or assembling multiple disks,
+                                 * when fdisk or wipefs is called, we trigger synthesized events and that can
+                                 * replace existing symlinks, and unexpected device may be used and data may
+                                 * be lost. */
+                                if (current_prio == prio &&
+                                    device_get_property_bool(dev, ".UDEV_REPLACE_SYMLINK_WITH_SAME_PRIORITY") <= 0)
+                                        return 0;
+
                                 /* This device has the equal or a higher priority than the current. Let's
                                  * create the devlink to our device node. */
                                 return node_create_symlink(dev, /* devnode = */ NULL, slink);

test/units/TEST-67-INTEGRITY.sh

@@ -34,6 +34,9 @@ ${DM_NAME} ${loop} - integrity-algorithm=$1
 EOF
 }
 
+udevadm settle
+udevadm control --log-level=debug
+
 image_dir="$(mktemp -d -t -p / integrity.tmp.XXXXXX)"
 if [ -z "${image_dir}" ] || [ ! -d "${image_dir}" ]; then
     echo "mktemp under / failed"
@@ -93,13 +96,7 @@ do
 
     # Check the signature on the FS to ensure we can retrieve it and that is matches
     if [ -e "${FULL_DM_DEV_NAME}" ]; then
-        # If a separate device is used for the metadata storage, then blkid will return one of the loop devices
-        if [ "${separate_data}" -eq 1 ]; then
-            dev_name="$(integritysetup status ${DM_NAME} | grep '^\s*device:' | awk '{print $2}')"
-        else
-            dev_name="${FULL_DM_DEV_NAME}"
-        fi
-        if [ "${dev_name}" != "$(blkid -U "${FS_UUID}")" ]; then
+        if [ "$(blkid -U "${FS_UUID}")" != "${FULL_DM_DEV_NAME}" ]; then
             echo "Failed to locate FS with matching UUID!"
             exit 1
         fi