From 56819864c0b391cbda861e4e3fce441efc2756bc Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Sat, 20 Jul 2024 21:56:15 +0200 Subject: [PATCH] libdevmapper: properly detect device busy failure for dm table devices Due to internal retry-overengineering in libdevmapper, some dm-ioctl failures can disappear. One such case is when there is a device creation race and DM device is created but reload fails. this can heppen because some block device used in table mapping is already claimed (it needs exclusive access for bdev_open in kernel). The kernel ioctl properly returns EBUSY, this errno is lost in libdevmapper (dm_task_get_errno returns 0). While this should be solved by libdevampper, we need some reliable way on older systems to properly report "busy" error instead of overloaded "invalid" error. With modified reproducer (see check_concurrent in very compat test), this situation can happen quite often. This patch modifies dm_create_device to return ENODEV only if dm-ioctl also reports no device (ENXIO); following dm status reports ENODEV and also some referenced device is no longer accesible through stat(). In all other cases we return EBUSY. Command line translates EBUSY and EEXIST to the same return vaules, for API users it now returns EBUSY instead of generic EINVAL. IOW, if device activation returns EEXIST or EBUSY, device-mapper cannot create the device because it already exits (EEXIST) or some referenced device is claimed by other subystem (EBUSY) and mapping table cannot be created. --- lib/libdevmapper.c | 72 ++++++++++++++++++++++++++++++++++++++-- tests/verity-compat-test | 1 + 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/lib/libdevmapper.c b/lib/libdevmapper.c index 086fb7ea..2e0d2ff4 100644 --- a/lib/libdevmapper.c +++ b/lib/libdevmapper.c @@ -1268,6 +1268,48 @@ err: return r; } +static bool device_disappeared(struct crypt_device *cd, struct device *device, const char *type) +{ + struct stat st; + + if (!device) + return false; + + /* + * Cannot use device_check_access(cd, device, DEV_OK) as it always accesses block device, + * we want to check for underlying file presence (if device is an image). + */ + if (stat(device_path(device), &st) < 0) { + log_dbg(cd, "%s device %s disappeared.", type, device_path(device)); + return true; + } + + log_dbg(cd, "%s device %s is OK.", type, device_path(device)); + return false; +} + +static bool dm_table_devices_disappeared(struct crypt_device *cd, struct crypt_dm_active_device *dmd) +{ + struct dm_target *tgt = &dmd->segment; + + do { + if (device_disappeared(cd, tgt->data_device, "Data")) + return true; + if (tgt->type == DM_VERITY) { + if (device_disappeared(cd, tgt->u.verity.hash_device, "Hash")) + return true; + if (device_disappeared(cd, tgt->u.verity.fec_device, "FEC")) + return true; + } else if (tgt->type == DM_INTEGRITY) { + if (device_disappeared(cd, tgt->u.integrity.meta_device, "Integrity meta")) + return true; + } + tgt = tgt->next; + } while (tgt); + + return false; +} + static int _dm_create_device(struct crypt_device *cd, const char *name, const char *type, struct crypt_dm_active_device *dmd) { @@ -1318,8 +1360,8 @@ static int _dm_create_device(struct crypt_device *cd, const char *name, const ch goto out; if (!dm_task_run(dmt)) { - r = -dm_task_get_errno(dmt); + log_dbg(cd, "DM create task failed, dm_task errno: %i.", r); if (r == -ENOKEY || r == -EKEYREVOKED || r == -EKEYEXPIRED) { /* propagate DM errors around key management as such */ r = -ENOKEY; @@ -1327,10 +1369,34 @@ static int _dm_create_device(struct crypt_device *cd, const char *name, const ch } r = dm_status_device(cd, name); - if (r >= 0) + log_dbg(cd, "Device status returned %i.", r); + if (r >= 0 || r == -EEXIST) { r = -EEXIST; - if (r != -EEXIST && r != -ENODEV) + goto out; + } + + /* EEXIST above has priority */ + if (dm_task_get_errno(dmt) == EBUSY) { + r = -EBUSY; + goto out; + } + + if (r != -ENODEV) { r = -EINVAL; + goto out; + } + + /* dm-ioctl failed => -ENODEV */ + if (dm_task_get_errno(dmt) == ENXIO) + goto out; + + /* Some device or file node disappeared => -ENODEV */ + if (dm_table_devices_disappeared(cd, dmd)) + goto out; + + /* Bail out with EBUSY better than sleep and retry. */ + log_dbg(cd, "No referenced device missing, some device in use."); + r = -EBUSY; goto out; } diff --git a/tests/verity-compat-test b/tests/verity-compat-test index 82c49d38..31e2faa2 100755 --- a/tests/verity-compat-test +++ b/tests/verity-compat-test @@ -423,6 +423,7 @@ function check_concurrent() # $1 hash $VERITYSETUP create -v $DEV_NAME $DEV_PARAMS $1 >>$DEV_OUT 2>&1 & wait grep -q "Command failed with code .* (wrong or missing parameters)" $DEV_OUT && fail + grep -q "Command failed with code .* (wrong device or file specified)." $DEV_OUT && fail check_exists rm $DEV_OUT $VERITYSETUP close $DEV_NAME >/dev/null 2>&1 || fail