[ndctl PATCH 0/4] libdaxctl: Fix multi-range memory block handling

[ndctl PATCH 0/4] libdaxctl: Fix multi-range memory block handling

[Alison Schofield]

[ Gregory: Hoping you can help w reviews here ]

Review and testing of Gregory's dax kmem atomic whole-device hotplug
series[1] exposed an existing libdaxctl bug when handling DAX devices with
multiple physical address ranges.

libdaxctl identifies a device's memory blocks using a single contiguous
address window, even when the device has separate ranges with gaps between
them. This can exclude blocks belonging to the device and include blocks
belonging to another device in the gaps. As a result, daxctl can report
incorrect memory block counts and potentially online or offline another
device's memory.

Patches 1-2 address mapping reliability issues discovered while fixing
the memory block ownership checks.

Patch 3 corrects the memory block ownership checks.

Patch 4 adds regression coverage using a two-range DAX device.

[1] https://lore.kernel.org/linux-cxl/20261016214248.443247-1-gourry@gourry.net/

Alison Schofield (4):
  libdaxctl: handle mapping initialization failures
  libdaxctl: tie mapping cache lifetime to the device
  libdaxctl: match memory blocks against each dax device mapping
  test/daxctl-create.sh: count memory blocks across dax mappings

 daxctl/lib/libdaxctl.c | 93 ++++++++++++++++++++++++++++++++++--------
 test/daxctl-create.sh  | 58 ++++++++++++++++++++++++++
 2 files changed, 135 insertions(+), 16 deletions(-)

-- 
2.37.3

[ndctl PATCH 1/4] libdaxctl: handle mapping initialization failures

[Alison Schofield]

A failed sysfs read can silently truncate a device's mapping list, leaving
callers with incomplete range information and potentially incorrect memory
block accounting. An allocation failure also causes an infinite loop,
hanging the caller.

Distinguish the end of the mapping list from read failures, reject
incomplete lists, and discard partial results on error. The mappings
describe the device's constituent ranges, so validate that their combined
size matches the device size. Return -ENOMEM on allocation failure.

Fixes: 5a31bf710399 ("libdaxctl: add mapping iterator APIs")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 daxctl/lib/libdaxctl.c | 70 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 58 insertions(+), 12 deletions(-)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 5b47c770bd15..f630f4791f59 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -222,6 +222,17 @@ static void free_mem(struct daxctl_dev *dev)
 	}
 }
 
+static void free_mappings(struct daxctl_dev *dev)
+{
+	struct daxctl_mapping *mapping, *_m;
+
+	list_for_each_safe(&dev->mappings, mapping, _m, list) {
+		list_del_from(&dev->mappings, &mapping->list);
+		free(mapping);
+	}
+	dev->num_mappings = -1;
+}
+
 static void free_dev(struct daxctl_dev *dev, struct list_head *head)
 {
 	if (head)
@@ -1269,15 +1280,17 @@ DAXCTL_EXPORT unsigned long daxctl_memory_get_block_size(struct daxctl_memory *m
 	return mem->block_size;
 }
 
-static void mappings_init(struct daxctl_dev *dev)
+static int mappings_init(struct daxctl_dev *dev)
 {
+	const char *devname = daxctl_dev_get_devname(dev);
 	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
+	unsigned long long size = 0;
 	char buf[SYSFS_ATTR_SIZE];
 	char *path = dev->dev_buf;
-	int i;
+	int i, rc;
 
 	if (dev->num_mappings != -1)
-		return;
+		return 0;
 
 	dev->num_mappings = 0;
 	for (;;) {
@@ -1285,31 +1298,43 @@ static void mappings_init(struct daxctl_dev *dev)
 		unsigned long long pgoff, start, end;
 
 		i = dev->num_mappings;
+		sprintf(path, "%s/mapping%d", dev->dev_path, i);
+		if (access(path, F_OK) < 0) {
+			if (errno == ENOENT)
+				break;
+			rc = -errno;
+			err(ctx, "%s: Unable to access mapping%d: %s\n",
+				devname, i, strerror(-rc));
+			goto out;
+		}
+
 		mapping = calloc(1, sizeof(*mapping));
 		if (!mapping) {
-			err(ctx, "%s: mapping%u allocation failure\n",
-				daxctl_dev_get_devname(dev), i);
-			continue;
+			rc = -ENOMEM;
+			goto err;
 		}
 
 		sprintf(path, "%s/mapping%d/start", dev->dev_path, i);
-		if (sysfs_read_attr(ctx, path, buf) < 0) {
+		rc = sysfs_read_attr(ctx, path, buf);
+		if (rc < 0) {
 			free(mapping);
-			break;
+			goto err;
 		}
 		start = strtoull(buf, NULL, 0);
 
 		sprintf(path, "%s/mapping%d/end", dev->dev_path, i);
-		if (sysfs_read_attr(ctx, path, buf) < 0) {
+		rc = sysfs_read_attr(ctx, path, buf);
+		if (rc < 0) {
 			free(mapping);
-			break;
+			goto err;
 		}
 		end = strtoull(buf, NULL, 0);
 
 		sprintf(path, "%s/mapping%d/page_offset", dev->dev_path, i);
-		if (sysfs_read_attr(ctx, path, buf) < 0) {
+		rc = sysfs_read_attr(ctx, path, buf);
+		if (rc < 0) {
 			free(mapping);
-			break;
+			goto err;
 		}
 		pgoff = strtoull(buf, NULL, 0);
 
@@ -1320,9 +1345,30 @@ static void mappings_init(struct daxctl_dev *dev)
 
 		dev->num_mappings++;
 		list_add(&dev->mappings, &mapping->list);
+		size += end - start + 1;
 	}
+
+	/* each of the device's ranges has a mappingN */
+	if (dev->num_mappings && size != dev->size) {
+		err(ctx, "%s: mappings total %#llx, device size %#llx\n",
+			devname, size, dev->size);
+		rc = -ENXIO;
+		goto out;
+	}
+
+	return 0;
+
+err:
+	/* sysfs_read_attr() already logged these */
+	if (rc != -EACCES && rc != -EPERM)
+		err(ctx, "%s: Unable to read mapping%d: %s\n", devname, i,
+			strerror(-rc));
+out:
+	free_mappings(dev);
+	return rc;
 }
 
+/* NULL means either no mappings or a failure to read them */
 DAXCTL_EXPORT struct daxctl_mapping *daxctl_mapping_get_first(struct daxctl_dev *dev)
 {
 	mappings_init(dev);
-- 
2.37.3

Re: [ndctl PATCH 1/4] libdaxctl: handle mapping initialization failures

[Gregory Price]

On 2026-10-22 12:56 -0700, Alison Schofield wrote:
> A failed sysfs read can silently truncate a device's mapping list, leaving
> callers with incomplete range information and potentially incorrect memory
> block accounting. An allocation failure also causes an infinite loop,
> hanging the caller.
> 
> Distinguish the end of the mapping list from read failures, reject
> incomplete lists, and discard partial results on error. The mappings
> describe the device's constituent ranges, so validate that their combined
> size matches the device size. Return -ENOMEM on allocation failure.
> 
> Fixes: 5a31bf710399 ("libdaxctl: add mapping iterator APIs")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Nothing stands out to me, this lgtm. Did a bit of testing as well.

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>
Tested-by: Gregory Price (Meta) <gourry@gourry.net>

-- 
Gregory Price (Meta) <gourry@gourry.net>

[ndctl PATCH 2/4] libdaxctl: tie mapping cache lifetime to the device

[Alison Schofield]

Cached mappings are not freed when a device is destroyed or invalidated
when its ranges change. This leaks memory and can leave callers using
obsolete physical address ranges, potentially operating on memory blocks
that no longer belong to the device.

Free mappings on device destruction and invalidate the cache when device
ranges may have changed.

Fixes: 5a31bf710399 ("libdaxctl: add mapping iterator APIs")
Fixes: 52a2653098a5 ("libdaxctl: add daxctl_dev_set_mapping()")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 daxctl/lib/libdaxctl.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index f630f4791f59..f7cbf3fa12e7 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -241,6 +241,7 @@ static void free_dev(struct daxctl_dev *dev, struct list_head *head)
 	free(dev->dev_buf);
 	free(dev->dev_path);
 	free_mem(dev);
+	free_mappings(dev);
 	free(dev);
 }
 
@@ -593,14 +594,15 @@ static void *add_dax_dev(void *parent, int id, const char *daxdev_base)
 	else
 		dev->target_node = -1;
 
+	dev->num_mappings = -1;
+	list_head_init(&dev->mappings);
+
 	daxctl_dev_foreach(region, dev_dup)
 		if (dev_dup->id == dev->id) {
 			free_dev(dev, NULL);
 			free(path);
 			return dev_dup;
 		}
-	dev->num_mappings = -1;
-	list_head_init(&dev->mappings);
 	list_add(&region->devices, &dev->list);
 	free(path);
 	return dev;
@@ -1186,6 +1188,7 @@ DAXCTL_EXPORT int daxctl_dev_set_size(struct daxctl_dev *dev, unsigned long long
 	}
 
 	sprintf(buf, "%#llx\n", size);
+	free_mappings(dev);
 	if (sysfs_write_attr(ctx, path, buf) < 0) {
 		err(ctx, "%s: failed to set size\n",
 				daxctl_dev_get_devname(dev));
@@ -1242,6 +1245,7 @@ DAXCTL_EXPORT int daxctl_dev_set_mapping(struct daxctl_dev *dev,
 	}
 
 	sprintf(buf, "%#llx-%#llx\n", start, end);
+	free_mappings(dev);
 	if (sysfs_write_attr(ctx, path, buf) < 0) {
 		err(ctx, "%s: failed to set mapping\n",
 				daxctl_dev_get_devname(dev));
-- 
2.37.3

Re: [ndctl PATCH 2/4] libdaxctl: tie mapping cache lifetime to the device

[Gregory Price]

On 2026-10-22 12:56 -0700, Alison Schofield wrote:
> Cached mappings are not freed when a device is destroyed or invalidated
> when its ranges change. This leaks memory and can leave callers using
> obsolete physical address ranges, potentially operating on memory blocks
> that no longer belong to the device.
> 
> Free mappings on device destruction and invalidate the cache when device
> ranges may have changed.
> 
> Fixes: 5a31bf710399 ("libdaxctl: add mapping iterator APIs")
> Fixes: 52a2653098a5 ("libdaxctl: add daxctl_dev_set_mapping()")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Looks right to me.  I'm a little light on the lifetime details for
the cached mappings, but i couldn't find any obvious issues.

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>

-- 
Gregory Price (Meta) <gourry@gourry.net>

[ndctl PATCH 3/4] libdaxctl: match memory blocks against each dax device mapping

[Alison Schofield]

memblock_in_dev() treats a DAX device as one contiguous physical address
range, even when the device has multiple ranges separated by gaps. This
misses blocks in later ranges and may incorrectly include blocks belonging
to another device.

Memory block counts and per-block online/offline operations use this
lookup, so daxctl may operate on memory outside the requested device.

Match blocks against each individual mapping, retaining the single-range
fallback for older kernels.

Fixes: e8bf803e359b ("libdaxctl: add a 'daxctl_memory' object for memory based operations")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 daxctl/lib/libdaxctl.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index f7cbf3fa12e7..b6b411dd916a 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -1615,6 +1615,7 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock)
 	const char *devname = daxctl_dev_get_devname(dev);
 	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
 	int rc, path_len = mem->buf_len;
+	struct daxctl_mapping *mapping;
 	unsigned long memblock_size;
 	char buf[SYSFS_ATTR_SIZE];
 	unsigned long phys_index;
@@ -1654,8 +1655,18 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock)
 	}
 	memblock_res = phys_index * memblock_size;
 
-	if (memblock_res >= dev_start && memblock_res <= dev_end)
-		return 1;
+	rc = mappings_init(dev);
+	if (rc < 0)
+		return rc;
+
+	/* kernels without mappingN report a single range */
+	if (list_empty(&dev->mappings))
+		return memblock_res >= dev_start && memblock_res <= dev_end;
+
+	daxctl_mapping_foreach(dev, mapping)
+		if (memblock_res >= mapping->start &&
+		    memblock_res <= mapping->end)
+			return 1;
 
 	return 0;
 }
-- 
2.37.3

Re: [ndctl PATCH 3/4] libdaxctl: match memory blocks against each dax device mapping

[Gregory Price]

On 2026-10-22 12:56 -0700, Alison Schofield wrote:
> memblock_in_dev() treats a DAX device as one contiguous physical address
> range, even when the device has multiple ranges separated by gaps. This
> misses blocks in later ranges and may incorrectly include blocks belonging
> to another device.
> 
> Memory block counts and per-block online/offline operations use this
> lookup, so daxctl may operate on memory outside the requested device.
> 
> Match blocks against each individual mapping, retaining the single-range
> fallback for older kernels.
> 
> Fixes: e8bf803e359b ("libdaxctl: add a 'daxctl_memory' object for memory based operations")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  daxctl/lib/libdaxctl.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>

Ran through an analysis and testing round with my clanker, seems to want
this fixup.  Looked reasonable, just passing it along.

Will get proper tags for you this week.

-- >8 --

Counting an unplugged device's blocks can return -ENOENT when its node
still has unrelated RAM. mappings_init() leaves ENOENT after probing
past the final mapping, and unrelated blocks take continue before the
scan resets errno. End-of-directory can preserve that stale error.

Clear errno immediately before each readdir() call so the final errno
check describes that read alone. Errors from block operations still
exit through out_dir with their saved return value.

This corrects "libdaxctl: match memory blocks against each dax device
mapping". Returning real node-directory entries with memory entries
last reproduces the error in the posted series and verifies the fix.

---
 daxctl/lib/libdaxctl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index e106fff..c154db9 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -1716,8 +1716,8 @@ static int daxctl_memory_op(struct daxctl_memory *mem, enum memory_op op)
 		return rc;
 	}
 
-	errno = 0;
-	while ((de = readdir(node_dir)) != NULL) {
+	/* Helpers may leave errno set even when a block is skipped. */
+	for (errno = 0; (de = readdir(node_dir)) != NULL; errno = 0) {
 		if (strncmp(de->d_name, "memory", 6) == 0) {
 			if (strncmp(de->d_name, "memory_", 7) == 0)
 				continue;
@@ -1734,7 +1734,6 @@ static int daxctl_memory_op(struct daxctl_memory *mem, enum memory_op op)
 			if (rc == 0)
 				count++;
 		}
-		errno = 0;
 	}
 
 	if (status_flags & MEM_ST_ZONE_INCONSISTENT)

-- 
Gregory Price (Meta) <gourry@gourry.net>

[ndctl PATCH 4/4] test/daxctl-create.sh: count memory blocks across dax mappings

[Alison Schofield]

Existing tests use single-range devices and cannot detect incorrect memory
block accounting across discontiguous mappings. A two-range device exposed
this bug by reporting only one of its two blocks.

Create a device with two separated mappings and verify its memory block
count against the expected count from both mappings.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 test/daxctl-create.sh | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
index 954a112d0f18..c4fe369eac3a 100755
--- a/test/daxctl-create.sh
+++ b/test/daxctl-create.sh
@@ -7,6 +7,9 @@ rc=77
 
 trap 'cleanup $LINENO' ERR
 
+policy=/sys/devices/system/memory/auto_online_blocks
+saved_policy=
+
 modprobe -r cxl_test
 modprobe cxl_test
 $CXL list
@@ -14,6 +17,7 @@ $CXL list
 cleanup()
 {
 	printf "Error at line %d\n" "$1"
+	[[ $saved_policy ]] && echo "$saved_policy" > "$policy"
 	[[ $testdev ]] && reset_dax
 	exit $rc
 }
@@ -396,6 +400,59 @@ daxctl_test7()
 	test_pass
 }
 
+# Test 8: multi-range system-ram
+# Successfully counts every memory block of a device whose two ranges
+# have a gap between them. The memory is never onlined: cxl_test memory
+# is not backed by RAM.
+daxctl_test8()
+{
+	local daxdev_1 daxdev_2 daxdev path m start end n nmaps=0 total=0
+	local bs size
+
+	bs=$((0x$(cat /sys/devices/system/memory/block_size_bytes)))
+	size=$((available / 4))
+	if (( size < bs || size % bs )); then
+		echo "test8: skipped, region too small for $bs byte memory blocks"
+		return 0
+	fi
+
+	# a devdax device in the gap between the two ranges, it has no
+	# memory blocks to be miscounted
+	daxdev_1=$("$DAXCTL" create-device -r "$region_id" -s $size | jq -er '.[].chardev')
+	daxdev_2=$("$DAXCTL" create-device -r "$region_id" -s $size | jq -er '.[].chardev')
+	test -n "$daxdev_2"
+	"$DAXCTL" disable-device "$daxdev_1" && "$DAXCTL" destroy-device "$daxdev_1"
+
+	daxdev=$("$DAXCTL" create-device -r "$region_id" -s $((size * 2)) | jq -er '.[].chardev')
+	test "$(daxctl_get_nr_mappings "$daxdev")" -eq 2
+
+	# memory blocks wholly inside each mapping
+	path=$(readlink -f /sys/bus/dax/devices/"$daxdev")
+	for m in "$path"/mapping[0-9]*; do
+		start=$(cat "$m"/start)
+		end=$(cat "$m"/end)
+		n=$(((end + 1) / bs - (start + bs - 1) / bs))
+		(( n > 0 ))
+		total=$((total + n))
+		nmaps=$((nmaps + 1))
+	done
+	test "$nmaps" -eq 2
+
+	saved_policy=$(cat "$policy")
+	echo offline > "$policy"
+	"$DAXCTL" reconfigure-device -N -m system-ram "$daxdev"
+	test "$("$DAXCTL" list -d "$daxdev" | jq -er '.[].total_memblocks')" -eq "$total"
+	"$DAXCTL" reconfigure-device -m devdax "$daxdev"
+	echo "$saved_policy" > "$policy"
+	saved_policy=
+
+	"$DAXCTL" disable-device -r "$region_id" all
+	"$DAXCTL" destroy-device -r "$region_id" all
+
+	clear_dev
+	test_pass
+}
+
 find_testdev
 rc=1
 setup_dev
@@ -407,6 +464,7 @@ daxctl_test4
 daxctl_test5
 daxctl_test6
 daxctl_test7
+daxctl_test8
 reset_dev
 modprobe -r cxl_test
 exit 0
-- 
2.37.3

Re: [ndctl PATCH 4/4] test/daxctl-create.sh: count memory blocks across dax mappings

[Gregory Price]

On 2026-10-22 12:56 -0700, Alison Schofield wrote:
> Existing tests use single-range devices and cannot detect incorrect memory
> block accounting across discontiguous mappings. A two-range device exposed
> this bug by reporting only one of its two blocks.
> 
> Create a device with two separated mappings and verify its memory block
> count against the expected count from both mappings.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

I think this misses an over-count scenario:

   A=dax0.1   create 512M
   B=dax0.2   create 512M
              destroy A
   C=dax0.1   create 1G  -> hole before B + space after B

140...15ff...  dax0.1
160...17ff...  dax0.2
180...19ff...  dax0.1

Not sure if you want to extend the test to add that or not, or if it's
out of scope.  But the test as-written otherwise seems ok.

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>

-- 
Gregory Price (Meta) <gourry@gourry.net>

Re: [ndctl PATCH 0/4] libdaxctl: Fix multi-range memory block handling

[Gregory Price (Meta)]

On 2026-10-22 12:56 -0700, Alison Schofield wrote:
> [ Gregory: Hoping you can help w reviews here ]
> 

in my queue, thanks for looking at this.

-- 
~Gregory