libmultipath: fix leaked device-mapper suspend after a failed remove

[suresh-thelkar]

Summary

Under a heavy device-mapper ioctl load (for example LUN-number recycling combined with ALUA transitions),
dm_flush_map__() could leave a multipath map suspended indefinitely after a failed DM_DEVICE_REMOVE.
A suspended map wedges any later task that touches it in uninterruptible D state, and because sync()
walks every block device, this hangs I/O node-wide until reboot.

This PR fixes two distinct defects on the failed-remove path in libmultipath/devmapper.c.

Background

Observed in production on a 6.6 kernel with a SAN multipath device under storage-reconfiguration churn that
triggers transient DM ioctl failures — LUN reassignment (LUN assignments … have changed) combined with ALUA
transitions. Storage itself was healthy — zero I/O errors and no application I/O in flight at teardown. The
defect is entirely in user space: multipath-tools left the map suspended after a failed remove. A
subsequent page-cache writeback then submitted a bio to the suspended map, which device-mapper holds without
completing or failing (expected behavior while suspended); the writeback task kept the folio lock and blocked
forever in D state, and sync_bdevs() spread the hang node-wide.

Problem

When DM_DEVICE_REMOVE fails, the map has already been suspended earlier in dm_flush_map__(). The old code
then:

1. Misclassified query errors as external removal. It treated anything that was not DM_IS_MPATH_YES as
   "removed externally" and returned DM_FLUSH_OK without resuming. But dm_is_mpath() also returns
   DM_IS_MPATH_ERR (-1) when the status query itself transiently fails under ioctl pressure. That error
   was read as a successful external removal, and the still-suspended map was abandoned.

2. Issued a fire-and-forget resume. Even on the path that did attempt recovery, it issued a single
   DM_DEVICE_RESUME whose result was ignored and never verified. If that resume did not take effect, the map
   stayed suspended.

Fix

Two commits:

1. libmultipath: dm_flush_map__: do not treat query error as external removal
   Only DM_IS_MPATH_NO now counts as "removed externally". A DM_IS_MPATH_ERR query result is logged and
   falls through to the resume path instead of being reported as success.

2. libmultipath: dm_flush_map__: verify the map resumed after a failed remove
   Add dm_resume_and_verify(), which re-issues DM_DEVICE_RESUME and confirms with dm_is_suspended() that
   the map is really active, retrying once. This is the same verify-resume pattern already used by
   dm_addmap_reload(), now applied to the flush/teardown path. If the map is still suspended after the
   retry, dm_flush_map__() returns DM_FLUSH_FAIL (still restoring queue_if_no_path) instead of
   reporting success, and a DM_IS_MPATH_ERR result no longer keeps retrying as if the map were present.

dm_resume_and_verify() also treats the case where another process removed the map between the failed remove
and the resume (suspended < 0 && dm_is_mpath() == DM_IS_MPATH_NO) as success, since that is not a leaked
suspend.

Impact

• Prevents node-wide I/O hangs caused by a leaked DM suspend on map teardown.
• Failed teardown is now reported accurately (DM_FLUSH_FAIL) so callers and monitoring can react, rather
  than being silently masked as success.
• No change to the success path or to behavior outside the failed-remove branch.

Files changed

• libmultipath/devmapper.c

Testing

• Built cleanly with gcc 13.3.0; full make produces multipath, multipathd, multipathc, and all
  libraries with no errors or warnings.
• libmultipath/devmapper.o recompiles warning-free under the project's flags.

Deterministic reproducer

Validated with a small LD_PRELOAD fault injector (dmfault.so) that, for a single test map, forces
DM_DEV_REMOVE → EBUSY and the following DM_TABLE_STATUS → error (so dm_is_mpath() returns -1) — the
exact misclassification condition. A fake scsi_debug disk is used (all real disks blacklisted); the injected
flush runs inside the multipath -f CLI process and the map's suspend state is then asserted.

Patched build → PASS:

flushing with injected fault: DM_DEV_REMOVE=EBUSY + dm_is_mpath()->-1
    [dmfault] forcing DM_DEV_REMOVE EBUSY for mpatha
    libdevmapper: device-mapper: remove ioctl on mpatha failed: Device or resource busy
    dm_simplecmd: libdm task=2 error: Device or resource busy
    [dmfault] forcing DM_TABLE_STATUS error for mpatha (dm_is_mpath -> -1)
    libdevmapper: device-mapper: status ioctl on mpatha failed: Input/output error
    mpatha: unable to verify map state after failed remove
    post-flush 'dmsetup info -o suspended mpatha' = 'Active'
[ PASS ] map is ACTIVE (resumed + verified) -> FIX WORKING (patched)

Evidence mapping:

Log line	Proves
forcing DM_DEV_REMOVE EBUSY for mpatha	remove failed → map suspended (the failed-remove condition)
forcing DM_TABLE_STATUS error ... (dm_is_mpath -> -1)	the DM_IS_MPATH_ERR misclassification trigger
mpatha: unable to verify map state after failed remove	Patch 1 — -1 no longer treated as "removed"; falls through to resume (this condlog did not exist before)
dmsetup info -o suspended mpatha = 'Active'	Patch 2 — dm_resume_and_verify() re-resumed and confirmed active

For contrast, an unpatched current release under the same injection leaves the map Suspended and the
harness reports [ FAIL ] map LEFT SUSPENDED after failed remove -> BUG REPRODUCED (unpatched), confirming
the defect is still present on the latest release.

dmfault.c — fault injector (full source)

Click to expand dmfault.c — build with gcc -shared -fPIC -o dmfault.so dmfault.c -ldl

/*
 * dmfault.c - deterministic fault injector for the failed-remove /
 * leaked-suspend test. LD_PRELOAD shim that reproduces the exact
 * dm_flush_map__() failure the patch guards, without needing a race:
 *
 *   1. The real DM_DEV_SUSPEND is allowed through -> the map is truly suspended.
 *   2. The DM_DEV_REMOVE ioctl for the target map is forced to fail with EBUSY
 *      (the map is NOT actually removed).
 *   3. The very next DM_TABLE_STATUS ioctl (exactly what dm_is_mpath() issues
 *      right after the failed remove) is forced to return -1, i.e. a query
 *      error.
 *
 * That drives dm_flush_map__() into the "dm_is_mpath() == DM_IS_MPATH_ERR" case:
 *   - unpatched: treats any non-YES result as "removed externally", returns
 *     success WITHOUT resuming  -> map left Suspended  (BUG)
 *   - patched  : an error is not a removal -> dm_resume_and_verify() runs
 *                -> map Active         (FIXED)
 *
 * Injection is gated by env vars so nothing happens unless requested, and is
 * scoped to a single map name so other DM activity is untouched:
 *   DMFAULT_MAP=<mapname>   only inject for this dm map (required)
 *   DMFAULT=1               enable injection (required)
 *
 * Build:  gcc -shared -fPIC -o dmfault.so dmfault.c -ldl
 * Use:    DMFAULT=1 DMFAULT_MAP=mpatha LD_PRELOAD=$PWD/dmfault.so multipath -f mpatha
 */
#define _GNU_SOURCE
#include <dlfcn.h>
#include <errno.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <linux/dm-ioctl.h>

static int (*real_ioctl)(int, unsigned long, ...);
static int fail_next_table;   /* one-shot: fail the next DM_TABLE_STATUS */

static const char *target_map(void)
{
	return getenv("DMFAULT_MAP");
}

static int enabled(void)
{
	const char *e = getenv("DMFAULT");
	return e && *e && *e != '0';
}

/* True if this dm_ioctl payload targets our map (by name). */
static int is_target(void *arg)
{
	struct dm_ioctl *dmi = (struct dm_ioctl *)arg;
	const char *want = target_map();

	if (!dmi || !want)
		return 0;
	/* dmi->name is NUL-terminated within the fixed-size field. */
	return strncmp(dmi->name, want, sizeof(dmi->name)) == 0;
}

int ioctl(int fd, unsigned long request, ...)
{
	va_list ap;
	void *arg;

	va_start(ap, request);
	arg = va_arg(ap, void *);
	va_end(ap);

	if (!real_ioctl)
		real_ioctl = dlsym(RTLD_NEXT, "ioctl");

	if (enabled() && _IOC_TYPE(request) == DM_IOCTL) {
		unsigned int cmd = _IOC_NR(request);

		if (cmd == DM_DEV_REMOVE_CMD && is_target(arg)) {
			/* Do NOT call the real remove: leave the map present
			 * but report failure, as if it were still busy. */
			fail_next_table = 1;
			fprintf(stderr,
				"[dmfault] forcing DM_DEV_REMOVE EBUSY for %s\n",
				target_map());
			errno = EBUSY;
			return -1;
		}
		if (cmd == DM_TABLE_STATUS_CMD && fail_next_table &&
		    is_target(arg)) {
			fail_next_table = 0;
			fprintf(stderr,
				"[dmfault] forcing DM_TABLE_STATUS error for %s "
				"(dm_is_mpath -> -1)\n", target_map());
			errno = EIO;
			return -1;
		}
	}

	return real_ioctl(fd, request, arg);
}

repro-dm-suspend.sh — test harness (full source)

Builds a multipath map over a fake scsi_debug disk (all real disks blacklisted, so it never touches
real storage), runs multipath -f under the injector, and asserts the map is not left suspended.

⚠️ Run this only in a disposable VM, as root. The harness loads scsi_debug/dm_multipath, writes a
temporary /etc/multipath.conf (backed up and restored on cleanup), and exercises a code path that can
leave a DM map suspended. On an unpatched build a leaked suspend can wedge sync() and hang I/O node-wide
until reboot — so never run it on a host you care about. Always run ./repro-dm-suspend.sh cleanup
afterwards.

Click to expand repro-dm-suspend.sh

#!/usr/bin/env bash
#
# repro-dm-suspend.sh - deterministic reproducer for the multipath-tools
# leaked-suspend-after-failed-remove bug.
#
# Root cause: during map teardown, dm_flush_map__() suspends the DM map, the
# DM_DEVICE_REMOVE fails with -EBUSY because the device is still open, and no
# successful DM_DEVICE_RESUME reaches the kernel -> the map is left SUSPENDED
# indefinitely. Any page-cache flush / sync() on that device then hangs forever
# in D state.
#
# The injector (dmfault.so, LD_PRELOAD) forces, for our map only, a failed
# DM_DEV_REMOVE followed by a DM_TABLE_STATUS error, i.e. dm_is_mpath() returns
# an error right after the failed remove - the exact misclassification:
#   unpatched -> "removed externally", no resume -> map left "Suspended" (FAIL)
#   patched   -> an error is not a removal -> resume+verify -> map "Active" (PASS)
#
# REQUIREMENTS: run as root, in a DISPOSABLE VM. Uses scsi_debug + dmsetup on
# FAKE devices only; it never touches real storage. The test asserts on the
# map's suspended state (no hang), and always resumes the map afterwards.
#
# Usage:
#   ./repro-dm-suspend.sh multipath-inject   # pre/post-patch assertion
#   ./repro-dm-suspend.sh cleanup
#
set -uo pipefail

DM_NAME="reprodmflush"
MPATH_CONF="/tmp/${DM_NAME}-multipath.conf"
MP_MAP=""

log()  { printf '\n\033[1;34m[repro]\033[0m %s\n' "$*"; }
ok()   { printf '\033[1;32m[ PASS ]\033[0m %s\n' "$*"; }
bad()  { printf '\033[1;31m[ FAIL ]\033[0m %s\n' "$*"; }
info() { printf '\033[0;36m       \033[0m %s\n' "$*"; }

require_root() {
	if [[ "$(id -u)" -ne 0 ]]; then
		echo "Must run as root (uses dmsetup / scsi_debug). Aborting." >&2
		exit 1
	fi
}

# Build a multipath map over a fake scsi_debug disk. Sets global MP_MAP.
# Real disks are untouched (blacklist all but the scsi_debug node).
mp_build_map() {
	require_root
	for bin in multipath dmsetup lsscsi; do
		command -v "$bin" >/dev/null 2>&1 || info "note: '$bin' not found (lsscsi optional)"
	done

	log "loading scsi_debug (fake SCSI disk)"
	modprobe scsi_debug dev_size_mb=64 add_host=1 num_tgts=1 vpd_use_hostno=0 || {
		bad "could not load scsi_debug"; return 1; }
	sleep 2

	local sd=""
	sd="$(lsscsi 2>/dev/null | awk '/scsi_debug/ {print $NF}' | head -1)"
	if [[ -z "$sd" ]]; then
		sd="/dev/$(ls -1 /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/*/block 2>/dev/null | head -1)"
	fi
	[[ -b "$sd" ]] || { bad "scsi_debug block device not found"; return 1; }
	info "scsi_debug device: $sd"

	local sd_base
	sd_base="$(basename "$sd")"

	# multipath only reads /etc/multipath.conf. Install a minimal config
	# (backed up + restored on cleanup) that blacklists everything EXCEPT the
	# scsi_debug node, so real disks are safe.
	if [[ -f /etc/multipath.conf && ! -f "${MPATH_CONF}.orig" ]]; then
		cp -a /etc/multipath.conf "${MPATH_CONF}.orig"
	fi
	cat > /etc/multipath.conf <<EOF
defaults {
    find_multipaths no
    user_friendly_names yes
}
blacklist {
    devnode ".*"
}
blacklist_exceptions {
    devnode "^${sd_base}\$"
}
EOF
	info "installed /etc/multipath.conf (only $sd_base whitelisted)"

	modprobe dm_multipath 2>/dev/null || true

	log "creating multipath map over $sd"
	multipath -a "$sd" 2>/dev/null || true
	multipath 2>/dev/null || true
	sleep 2
	multipath -ll 2>/dev/null | sed 's/^/         /'

	local map=""
	map="$(dmsetup ls --target multipath 2>/dev/null | awk 'NR==1{print $1}')"
	if [[ -z "$map" || "$map" == "No" ]]; then
		bad "no multipath map was created (check that scsi_debug got claimed)"
		return 1
	fi
	info "multipath map: $map"
	MP_MAP="$map"
	return 0
}

# Deterministic before/after test via fault injection.
multipath_inject() {
	require_root
	log "deterministic fault-injection before/after test"

	local so="${DMFAULT_SO:-$(dirname "$(readlink -f "$0")")/dmfault.so}"
	if [[ ! -f "$so" ]]; then
		bad "dmfault.so not found at $so"
		info "build it: gcc -shared -fPIC -o dmfault.so dmfault.c -ldl"
		return 1
	fi
	info "using injector: $so"

	mp_build_map || return 1
	local map="$MP_MAP"

	log "flushing with injected fault: DM_DEV_REMOVE=EBUSY + dm_is_mpath()->error"
	DMFAULT=1 DMFAULT_MAP="$map" LD_PRELOAD="$so" \
		timeout 30 multipath -f "$map" 2>&1 | sed 's/^/         /' || true
	sleep 1

	if ! dmsetup info "$map" &>/dev/null; then
		info "map is gone - injection did not take effect; inspect manually"
		return 0
	fi
	local suspended
	suspended="$(dmsetup info -c --noheadings -o suspended "$map" 2>/dev/null | tr -d ' ')"
	info "post-flush 'dmsetup info -o suspended $map' = '$suspended'"
	if [[ "$suspended" == "Suspended" ]]; then
		bad "map LEFT SUSPENDED after failed remove -> BUG REPRODUCED (unpatched)"
	elif [[ "$suspended" == "Active" ]]; then
		ok "map is ACTIVE (resumed + verified) -> FIX WORKING (patched)"
	else
		info "unexpected suspend state '$suspended' - inspect manually"
	fi

	# Never leave a suspended map behind.
	dmsetup resume "$map" 2>/dev/null || true
}

multipath_cleanup() {
	require_root
	log "cleanup: tearing down reproducer resources"
	if [[ -n "${MP_MAP:-}" ]] && dmsetup info "$MP_MAP" &>/dev/null; then
		dmsetup resume "$MP_MAP" 2>/dev/null
		multipath -f "$MP_MAP" 2>/dev/null || dmsetup remove --force "$MP_MAP" 2>/dev/null
	fi
	multipath -F 2>/dev/null || true
	modprobe -r scsi_debug 2>/dev/null || true
	if [[ -f "${MPATH_CONF}.orig" ]]; then
		mv -f "${MPATH_CONF}.orig" /etc/multipath.conf
	else
		rm -f /etc/multipath.conf 2>/dev/null
	fi
	rm -f "$MPATH_CONF"
	ok "cleanup done"
}

trap 'multipath_cleanup' EXIT

case "${1:-}" in
	multipath-inject) multipath_inject ;;
	cleanup)          trap - EXIT; multipath_cleanup ;;
	*)
		cat <<EOF
Deterministic reproducer for the leaked-suspend-after-failed-remove bug.

Commands:
  multipath-inject   before/after test: uses dmfault.so to force the exact
                     failed-remove + dm_is_mpath()->error condition.
                       unpatched -> [ FAIL ] map left Suspended
                       patched   -> [ PASS ] map Active
  cleanup            Remove all reproducer resources.

Run as root in a DISPOSABLE VM. Build the injector first:
  gcc -shared -fPIC -o dmfault.so dmfault.c -ldl
EOF
		;;
esac

[suresh-thelkar]

Hi @mwilck @bmarzins — flagging this for review when you have a chance.

Small two-commit fix in libmultipath/devmapper.c for a leaked device-mapper suspend on the
failed-remove path in dm_flush_map__(): a transient dm_is_mpath() query error was
misclassified as an external removal (so the map was never resumed), and the recovery resume
was fire-and-forget. Left unfixed, the map stays suspended and a later sync()/writeback
hangs node-wide. The commits (1) stop treating a query error as a removal and (2) add
dm_resume_and_verify(), mirroring the existing verify-resume pattern in dm_addmap_reload().
Based on current master, Signed-off-by, and git clang-format-clean on the touched lines;
a deterministic reproducer is in the description.

Happy to send the series to dm-devel@lists.linux.dev (Cc you both) if you'd prefer the
mailing-list flow. First contribution here — glad to adjust anything on process or style.

Thanks!