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.
This commit is contained in:
Milan Broz
2024-07-20 21:56:15 +02:00
parent 6af5e98792
commit 56819864c0
2 changed files with 70 additions and 3 deletions

View File

@@ -1268,6 +1268,48 @@ err:
return r; 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, static int _dm_create_device(struct crypt_device *cd, const char *name, const char *type,
struct crypt_dm_active_device *dmd) 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; goto out;
if (!dm_task_run(dmt)) { if (!dm_task_run(dmt)) {
r = -dm_task_get_errno(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) { if (r == -ENOKEY || r == -EKEYREVOKED || r == -EKEYEXPIRED) {
/* propagate DM errors around key management as such */ /* propagate DM errors around key management as such */
r = -ENOKEY; 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); r = dm_status_device(cd, name);
if (r >= 0) log_dbg(cd, "Device status returned %i.", r);
if (r >= 0 || r == -EEXIST) {
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; 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; goto out;
} }

View File

@@ -423,6 +423,7 @@ function check_concurrent() # $1 hash
$VERITYSETUP create -v $DEV_NAME $DEV_PARAMS $1 >>$DEV_OUT 2>&1 & $VERITYSETUP create -v $DEV_NAME $DEV_PARAMS $1 >>$DEV_OUT 2>&1 &
wait wait
grep -q "Command failed with code .* (wrong or missing parameters)" $DEV_OUT && fail 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 check_exists
rm $DEV_OUT rm $DEV_OUT
$VERITYSETUP close $DEV_NAME >/dev/null 2>&1 || fail $VERITYSETUP close $DEV_NAME >/dev/null 2>&1 || fail