Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [PATCH 00/12] btrfs-progs: image: various LLM found memory
@ 2026-10-25  8:41 Qu Wenruo
  2026-10-25  8:41 ` [PATCH 01/12] btrfs-progs: image: destroy chunk tree after all pthreads finished Qu Wenruo
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

The btrfs-image code base is pretty old (and to be honest, not written to
be read easily), and some stupid LLM-script-kiddies are reporting those
as security problems.

I don't want to argue with those guys, so just fix obvious memory related
problems for now.

There are still some left, mostly related to the corrupted tree
blocks handling, those will be addressed later, as I'm still trying to
figure out what's the best way to address them (e.g. tree-checker or
adhoc fixes).

There is also a problem in the filename image/sanitize.c, but that only
affects the filename sanitization, thus it's a very low priority item.

Qu Wenruo (12):
  btrfs-progs: image: destroy chunk tree after all pthreads finished
  btrfs-progs: image: only remove a chunk map from physical tree if
    non-empty
  btrfs-progs: image: cleanup pending ordered works after an error
  btrfs-progs: image: do not cleanup the thread that failed to be
    created
  btrfs-progs: image: free the buffer if output allocation failed
  btrfs-progs: image: avoid NULL-ptr-deref in build_chunk_tree()
  btrfs-progs: image: fix the error handling of build_chunk_tree()
  btrfs-progs: image: fix memory leak on async_work for restore
  btrfs-progs: image: fix overwritten ret in restore_metadump()
  btrfs-progs: image: validate the nritems before reading the content
  btrfs-progs: image: reject incorrect super block size
  btrfs-progs: image: properly error out when fseek() failed

 image/image-create.c  |  22 ++++++---
 image/image-restore.c | 109 +++++++++++++++++++++++++++++++++---------
 image/metadump.h      |   7 ++-
 3 files changed, 108 insertions(+), 30 deletions(-)

--
2.56.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/12] btrfs-progs: image: destroy chunk tree after all pthreads finished
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 02/12] btrfs-progs: image: only remove a chunk map from physical tree if non-empty Qu Wenruo
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

In the function mdrestore_destroy(), the chunk tree is destroyed before
all the pthreads finished.

This can lead to use-after-free problems for the chunk tree.

Fix it by waiting for all pthreads before destroy the chunk tree.

Since we're here, also fix the mdrestore_destroy() call inside
mdrestore_init() where if an error happened, we wait for a thread that
failed to be created.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/image/image-restore.c b/image/image-restore.c
index 125ac06f339b..9bc11ea5e7d9 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -50,15 +50,6 @@ static void mdrestore_destroy(struct mdrestore_struct *mdres, int num_threads)
 	struct rb_node *n;
 	int i;
 
-	while ((n = rb_first(&mdres->chunk_tree))) {
-		struct fs_chunk *entry;
-
-		entry = rb_entry(n, struct fs_chunk, l);
-		rb_erase(n, &mdres->chunk_tree);
-		rb_erase(&entry->p, &mdres->physical_tree);
-		free(entry);
-	}
-	free_extent_cache_tree(&mdres->sys_chunks);
 	pthread_mutex_lock(&mdres->mutex);
 	mdres->done = 1;
 	pthread_cond_broadcast(&mdres->cond);
@@ -69,6 +60,16 @@ static void mdrestore_destroy(struct mdrestore_struct *mdres, int num_threads)
 
 	pthread_cond_destroy(&mdres->cond);
 	pthread_mutex_destroy(&mdres->mutex);
+
+	while ((n = rb_first(&mdres->chunk_tree))) {
+		struct fs_chunk *entry;
+
+		entry = rb_entry(n, struct fs_chunk, l);
+		rb_erase(n, &mdres->chunk_tree);
+		rb_erase(&entry->p, &mdres->physical_tree);
+		free(entry);
+	}
+	free_extent_cache_tree(&mdres->sys_chunks);
 	free(mdres->original_super);
 }
 
@@ -646,7 +647,7 @@ static int mdrestore_init(struct mdrestore_struct *mdres,
 		}
 	}
 	if (ret)
-		mdrestore_destroy(mdres, i + 1);
+		mdrestore_destroy(mdres, i);
 	return ret;
 }
 
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/12] btrfs-progs: image: only remove a chunk map from physical tree if non-empty
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
  2026-10-25  8:41 ` [PATCH 01/12] btrfs-progs: image: destroy chunk tree after all pthreads finished Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 03/12] btrfs-progs: image: cleanup pending ordered works after an error Qu Wenruo
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

During read_chunk_tree_block(), if a chunk is already in the physical
tree, then the fs_chunk is only added to overlapping_chunks list, but
not yet inserted into the physical_tree, since inserting it would
conflict with the existing chunk

Then remap_overlapping_chunks() remap the chunk into a new location, and
re-insert the updated fs_chunk into physical tree.

However if some error happened before remap_overlapping_chunks() is
called, one fs_chunk can be linked to overlapping_chunks list, but its
rbtree entry is not properly initialized (all zero), then
mdrestore_destroy() still calls rb_erase() on the uninitialized entry.

Fix the problem by always clearing the allocated fs_chunk->p.
Then at mdrestore_destroy() only call rb_erase() if the rb tree pointer
is not empty.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/image/image-restore.c b/image/image-restore.c
index 9bc11ea5e7d9..b1de96c51efa 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -66,7 +66,8 @@ static void mdrestore_destroy(struct mdrestore_struct *mdres, int num_threads)
 
 		entry = rb_entry(n, struct fs_chunk, l);
 		rb_erase(n, &mdres->chunk_tree);
-		rb_erase(&entry->p, &mdres->physical_tree);
+		if (!RB_EMPTY_NODE(&entry->p))
+			rb_erase(&entry->p, &mdres->physical_tree);
 		free(entry);
 	}
 	free_extent_cache_tree(&mdres->sys_chunks);
@@ -806,6 +807,7 @@ static int read_chunk_tree_block(struct mdrestore_struct *mdres,
 		fs_chunk->physical = btrfs_stripe_offset_nr(eb, chunk, 0);
 		fs_chunk->bytes = btrfs_chunk_length(eb, chunk);
 		INIT_LIST_HEAD(&fs_chunk->list);
+		RB_CLEAR_NODE(&fs_chunk->p);
 
 		if (tree_search(&mdres->physical_tree, &fs_chunk->p,
 				physical_cmp, 1) != NULL)
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/12] btrfs-progs: image: cleanup pending ordered works after an error
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
  2026-10-25  8:41 ` [PATCH 01/12] btrfs-progs: image: destroy chunk tree after all pthreads finished Qu Wenruo
  2026-10-25  8:41 ` [PATCH 02/12] btrfs-progs: image: only remove a chunk map from physical tree if non-empty Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 04/12] btrfs-progs: image: do not cleanup the thread that failed to be created Qu Wenruo
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

The function flush_pending() can queue at most ITEMS_PER_CLUSTER
async_works before submitting them to write_buffers().

But if we hit an critical error, e.g. read_tree_block() hit a bad tree
block, and there is already pending async_works, we rely on the last
flush_pending() call inside create_metadump() to do the cleanup.

However that final flush_pending() will hit the same corrupted tree
block and fail halfway, without really dispatch the existing async
works.

Fix the problem by:

- Avoid calling the final flush_pending() if we had already hit an error

- Manually cleanup any pending async works inside metadump_destroy()

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-create.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/image/image-create.c b/image/image-create.c
index d172fcd3897a..1fd288d2b566 100644
--- a/image/image-create.c
+++ b/image/image-create.c
@@ -124,6 +124,14 @@ static void metadump_destroy(struct metadump_struct *md, int num_threads)
 	pthread_cond_destroy(&md->cond);
 	pthread_mutex_destroy(&md->mutex);
 
+	while (!list_empty(&md->ordered)) {
+		struct async_work *async = list_first_entry(&md->ordered,
+				struct async_work, ordered);
+
+		list_del_init(&async->ordered);
+		free(async->buffer);
+		free(async);
+	}
 	while ((n = rb_first(&md->name_tree))) {
 		struct name *name;
 
@@ -828,13 +836,14 @@ int create_metadump(const char *input, FILE *out, int num_threads,
 
 	ret = copy_space_cache(root, &metadump, &path);
 out:
-	ret = flush_pending(&metadump, 1);
-	if (ret) {
-		if (!err)
-			err = ret;
-		error("failed to flush pending data: %d", ret);
+	if (!ret) {
+		ret = flush_pending(&metadump, 1);
+		if (ret)
+			error("failed to flush pending data: %d", ret);
 	}
 
+	if (ret && !err)
+		err = ret;
 	metadump_destroy(&metadump, num_threads);
 
 	btrfs_release_path(&path);
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/12] btrfs-progs: image: do not cleanup the thread that failed to be created
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (2 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 03/12] btrfs-progs: image: cleanup pending ordered works after an error Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 05/12] btrfs-progs: image: free the buffer if output allocation failed Qu Wenruo
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

Inside metadump_init(), we initialize every thread slot, but when one
failed, we need to cleanup the previously created one.

However the existing code tries to clean up the slot that failed the
pthread_create() call.
This means in metadump_destroy() we call pthread_join() on the slot where
it's still all zero.

Fix the error handling by only releasing the allocated slots.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-create.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/image/image-create.c b/image/image-create.c
index 1fd288d2b566..75f450520059 100644
--- a/image/image-create.c
+++ b/image/image-create.c
@@ -179,7 +179,7 @@ static int metadump_init(struct metadump_struct *md, struct btrfs_root *root,
 	}
 
 	if (ret)
-		metadump_destroy(md, i + 1);
+		metadump_destroy(md, i);
 
 	return ret;
 }
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/12] btrfs-progs: image: free the buffer if output allocation failed
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (3 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 04/12] btrfs-progs: image: do not cleanup the thread that failed to be created Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 06/12] btrfs-progs: image: avoid NULL-ptr-deref in build_chunk_tree() Qu Wenruo
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

Inside dump_worker(), if the compression output buffer allocation
failed, since async->buffer is reused, no one can reach the old original
buffer.

This leads to leaked memory on the error path. Release the original
buffer in that case.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-create.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/image/image-create.c b/image/image-create.c
index 75f450520059..97348f37acaa 100644
--- a/image/image-create.c
+++ b/image/image-create.c
@@ -68,6 +68,7 @@ static void *dump_worker(void *data)
 			async->bufsize = compressBound(async->size);
 			async->buffer = malloc(async->bufsize);
 			if (!async->buffer) {
+				free(orig);
 				error_mem("async buffer");
 				pthread_mutex_lock(&md->mutex);
 				if (!md->error)
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/12] btrfs-progs: image: avoid NULL-ptr-deref in build_chunk_tree()
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (4 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 05/12] btrfs-progs: image: free the buffer if output allocation failed Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 07/12] btrfs-progs: image: fix the error handling of build_chunk_tree() Qu Wenruo
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

Although very unlikely, if there is a header that has no item in it,
build_chunk_tree() left @item as NULL, then later during the error
message we still call "get_unaligned_le64(&item->bytenr)", which is
going to trigger a NULL-pointer dereference.

Fix it by removing the get_unaligned_le64() call.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/image/image-restore.c b/image/image-restore.c
index b1de96c51efa..89ce2f3e79ed 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -1140,7 +1140,7 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 	}
 
 	if (!item || get_unaligned_le64(&item->bytenr) != BTRFS_SUPER_INFO_OFFSET) {
-		error("did not find superblock at %llu", get_unaligned_le64(&item->bytenr));
+		error("did not find superblock");
 		return -EINVAL;
 	}
 
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/12] btrfs-progs: image: fix the error handling of build_chunk_tree()
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (5 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 06/12] btrfs-progs: image: avoid NULL-ptr-deref in build_chunk_tree() Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 08/12] btrfs-progs: image: fix memory leak on async_work for restore Qu Wenruo
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

When btrfs_check_super() or add_sys_array() failed, the buffer is not
released and the pthread mutex is still locked.

Fix it by adding a new out label, to release the buffer, unlock the
mutex and return the error code.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/image/image-restore.c b/image/image-restore.c
index 89ce2f3e79ed..2e4c19746cfb 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -1182,14 +1182,12 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 	ret = btrfs_check_super(super, 0);
 	if (ret < 0) {
 		error("invalid superblock");
-		return ret;
+		goto out;
 	}
 	ret = add_sys_array(mdres, super);
 	if (ret < 0) {
 		error("failed to read system chunk array");
-		free(buffer);
-		pthread_mutex_unlock(&mdres->mutex);
-		return ret;
+		goto out;
 	}
 	mdres->nodesize = btrfs_super_nodesize(super);
 	if (btrfs_super_incompat_flags(super) &
@@ -1200,9 +1198,12 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 
 	memcpy(mdres->uuid, super->dev_item.uuid, BTRFS_UUID_SIZE);
 	mdres->devid = get_unaligned_le64(&super->dev_item.devid);
+out:
 	free(buffer);
 	pthread_mutex_unlock(&mdres->mutex);
 
+	if (ret < 0)
+		return ret;
 	return search_for_chunk_blocks(mdres);
 }
 
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/12] btrfs-progs: image: fix memory leak on async_work for restore
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (6 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 07/12] btrfs-progs: image: fix the error handling of build_chunk_tree() Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 09/12] btrfs-progs: image: fix overwritten ret in restore_metadump() Qu Wenruo
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

There are two paths to leak the async_work memory, fix them by:

- restore_one_work() failure inside restore_worker()
  Manually release memory before jumping to out label.

- all restore_worker() failed to process any async_work
  Those async_work will stay inside mdres->list until the end of the
  program.

  Fix them by manually drain the mdres->list.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/image/image-restore.c b/image/image-restore.c
index 2e4c19746cfb..4929beeb697e 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -61,6 +61,14 @@ static void mdrestore_destroy(struct mdrestore_struct *mdres, int num_threads)
 	pthread_cond_destroy(&mdres->cond);
 	pthread_mutex_destroy(&mdres->mutex);
 
+	while (!list_empty(&mdres->list)) {
+		struct async_work *async = list_first_entry(&mdres->list,
+						struct async_work, list);
+
+		list_del_init(&async->list);
+		free(async->buffer);
+		free(async);
+	}
 	while ((n = rb_first(&mdres->chunk_tree))) {
 		struct fs_chunk *entry;
 
@@ -590,6 +598,8 @@ static void *restore_worker(void *data)
 		if (ret < 0) {
 			mdres->error = ret;
 			pthread_mutex_unlock(&mdres->mutex);
+			free(async->buffer);
+			free(async);
 			goto out;
 		}
 		mdres->num_items--;
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/12] btrfs-progs: image: fix overwritten ret in restore_metadump()
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (7 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 08/12] btrfs-progs: image: fix memory leak on async_work for restore Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 10/12] btrfs-progs: image: validate the nritems before reading the content Qu Wenruo
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

In the "while(!mdrestore.error) {}" loop if we got a bad header or
add_cluster() failed, we break the loop.

But immediately after breaking out the loop, we call wait_for_worker(),
which overwrites @ret, and also continue to open_ctree().

Fix the problem by directly jumping to out label.
Also for bad header case, update the error code to -EUCLEAN.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/image/image-restore.c b/image/image-restore.c
index 4929beeb697e..3f2a7fcf797c 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -1852,13 +1852,13 @@ int restore_metadump(const char *input, FILE *out, int old_restore,
 		if (get_unaligned_le64(&header->magic) != current_version->magic_cpu ||
 		    get_unaligned_le64(&header->bytenr) != bytenr) {
 			error("bad header in metadump image");
-			ret = -EIO;
-			break;
+			ret = -EUCLEAN;
+			goto out;
 		}
 		ret = add_cluster(cluster, &mdrestore, &bytenr);
 		if (ret) {
 			error("failed to add cluster: %d", ret);
-			break;
+			goto out;
 		}
 	}
 	ret = wait_for_worker(&mdrestore);
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/12] btrfs-progs: image: validate the nritems before reading the content
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (8 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 09/12] btrfs-progs: image: fix overwritten ret in restore_metadump() Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 11/12] btrfs-progs: image: reject incorrect super block size Qu Wenruo
  2026-10-25  8:41 ` [PATCH 12/12] btrfs-progs: image: properly error out when fseek() failed Qu Wenruo
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

When iterating a header, we read out one 1KiB buffer, then grab the
cluster->header->nritems to grab the number of items stored in the 1K
block.

However if a corrupted or maliciously crafted header is passed in, we
can easily grab a huge number as @nritems, and easily go beyond the 1KiB
buffer.

Fix it by rejecting obviously incorrect nritems before iterating, this
involves 3 call sites:

- build_chunk_tree()
- search_for_chunk_blocks()
- add_cluster()

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/image/image-restore.c b/image/image-restore.c
index 3f2a7fcf797c..525cdd4d32bc 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -1030,6 +1030,12 @@ static int search_for_chunk_blocks(struct mdrestore_struct *mdres)
 
 		bytenr += IMAGE_BLOCK_SIZE;
 		nritems = get_unaligned_le32(&header->nritems);
+		if (nritems > ITEMS_PER_CLUSTER) {
+			error("invalid nritems, has %u expect <= %zu",
+			      nritems, ITEMS_PER_CLUSTER);
+			ret = -EUCLEAN;
+			goto out;
+		}
 
 		/* Search items for tree blocks in sys chunks */
 		for (i = 0; i < nritems; i++) {
@@ -1138,6 +1144,11 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 
 	mdres->compress_method = header->compress;
 	nritems = get_unaligned_le32(&header->nritems);
+	if (nritems > ITEMS_PER_CLUSTER) {
+		error("invalid nritems, has %u expect <= %zu",
+		      nritems, ITEMS_PER_CLUSTER);
+		return -EUCLEAN;
+	}
 	for (i = 0; i < nritems; i++) {
 		item = &cluster->items[i];
 
@@ -1316,6 +1327,11 @@ static int add_cluster(struct meta_cluster *cluster,
 
 	bytenr = get_unaligned_le64(&header->bytenr) + IMAGE_BLOCK_SIZE;
 	nritems = get_unaligned_le32(&header->nritems);
+	if (nritems > ITEMS_PER_CLUSTER) {
+		error("invalid nritems, has %u expect <= %zu",
+		      nritems, ITEMS_PER_CLUSTER);
+		return -EUCLEAN;
+	}
 	for (i = 0; i < nritems; i++) {
 		item = &cluster->items[i];
 		async = calloc(1, sizeof(*async));
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/12] btrfs-progs: image: reject incorrect super block size
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (9 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 10/12] btrfs-progs: image: validate the nritems before reading the content Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  2026-10-25  8:41 ` [PATCH 12/12] btrfs-progs: image: properly error out when fseek() failed Qu Wenruo
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

For an uncompressed image, if the super block size is smaller than
BTRFS_SUPER_INFO_SIZE, we know it's definitely corrupted, reject it or
later code can read beyond the allocated buffer.

This covers the following cases:

- build_chunk_tree()
  * Validate the compression level
  * Check the uncompressed super block size
  * Handle short decompression

- add_cluster()
  * Validate the compression level

- fill_mdres_info()
  if build_chunk_tree() is skipped, we read super block here.

  * Check the uncompressed super block size
  * Call btrfs_check_super() to validate the super block
  * Handle short decompression

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 42 ++++++++++++++++++++++++++++++++++++++----
 image/metadump.h      |  7 +++++--
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/image/image-restore.c b/image/image-restore.c
index 525cdd4d32bc..7f3c9fdb6536 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -1141,6 +1141,11 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 		error("bad header in metadump image");
 		return -EIO;
 	}
+	if (header->compress >= COMPRESS_LAST) {
+		error("unsupported compression, has %u expect < %u",
+		      header->compress, COMPRESS_LAST);
+		return -EUCLEAN;
+	}
 
 	mdres->compress_method = header->compress;
 	nritems = get_unaligned_le32(&header->nritems);
@@ -1165,6 +1170,17 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 		return -EINVAL;
 	}
 
+	/*
+	 * The super block may get merged with the next block. So here
+	 * only reject uncompressed size that is smaller than a sb.
+	 */
+	if (mdres->compress_method == COMPRESS_NONE &&
+	    get_unaligned_le32(&item->size) < BTRFS_SUPER_INFO_SIZE) {
+		error("incorrect super block size, has %u expect >= %u",
+		      get_unaligned_le32(&item->size), BTRFS_SUPER_INFO_SIZE);
+		return -EUCLEAN;
+	}
+
 	buffer = malloc(get_unaligned_le32(&item->size));
 	if (!buffer) {
 		error_mem(NULL);
@@ -1188,8 +1204,8 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 			return -ENOMEM;
 		}
 		ret = uncompress(tmp, (unsigned long *)&size, buffer, get_unaligned_le32(&item->size));
-		if (ret != Z_OK) {
-			error("decompression failed with %d", ret);
+		if (ret != Z_OK || size < BTRFS_SUPER_INFO_SIZE) {
+			error("decompression failed or short result");
 			free(buffer);
 			free(tmp);
 			return -EIO;
@@ -1288,17 +1304,29 @@ static int fill_mdres_info(struct mdrestore_struct *mdres,
 			return -ENOMEM;
 		ret = uncompress(buffer, (unsigned long *)&size,
 				 async->buffer, async->bufsize);
-		if (ret != Z_OK) {
-			error("decompression failed with %d", ret);
+		if (ret != Z_OK || size < BTRFS_SUPER_INFO_SIZE) {
+			error("decompression failed or short result");
 			free(buffer);
 			return -EIO;
 		}
 		outbuf = buffer;
 	} else {
 		outbuf = async->buffer;
+		/* The same check in build_chunk_tree(). */
+		if (async->bufsize < BTRFS_SUPER_INFO_SIZE) {
+			error("incorrect super block size, has %zu expect >= %u",
+			      async->bufsize, BTRFS_SUPER_INFO_SIZE);
+			return -EUCLEAN;
+		}
 	}
 
 	super = (struct btrfs_super_block *)outbuf;
+	ret = btrfs_check_super(super, 0);
+	if (ret < 0) {
+		free(buffer);
+		error("invalid superblock");
+		return ret;
+	}
 	mdres->nodesize = btrfs_super_nodesize(super);
 	if (btrfs_super_incompat_flags(super) &
 	    BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
@@ -1322,6 +1350,12 @@ static int add_cluster(struct meta_cluster *cluster,
 	int ret;
 
 	pthread_mutex_lock(&mdres->mutex);
+	if (header->compress >= COMPRESS_LAST) {
+		pthread_mutex_unlock(&mdres->mutex);
+		error("unsupported compression, has %u expect < %u",
+		      header->compress, COMPRESS_LAST);
+		return -EUCLEAN;
+	}
 	mdres->compress_method = header->compress;
 	pthread_mutex_unlock(&mdres->mutex);
 
diff --git a/image/metadump.h b/image/metadump.h
index a4c5d05ed1dd..3a7e38f82a8e 100644
--- a/image/metadump.h
+++ b/image/metadump.h
@@ -30,8 +30,11 @@
 #define ITEMS_PER_CLUSTER ((IMAGE_BLOCK_SIZE - sizeof(struct meta_cluster)) / \
 			   sizeof(struct meta_cluster_item))
 
-#define COMPRESS_NONE		0
-#define COMPRESS_ZLIB		1
+enum image_compression {
+	COMPRESS_NONE,
+	COMPRESS_ZLIB,
+	COMPRESS_LAST,
+};
 
 #define MAX_WORKER_THREADS	(32)
 
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 12/12] btrfs-progs: image: properly error out when fseek() failed
  2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
                   ` (10 preceding siblings ...)
  2026-10-25  8:41 ` [PATCH 11/12] btrfs-progs: image: reject incorrect super block size Qu Wenruo
@ 2026-10-25  8:41 ` Qu Wenruo
  11 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2026-10-25  8:41 UTC (permalink / raw)
  To: linux-btrfs

When fseek() inside restore_metadump() failed, @ret is still 0, and no
error is returned.

Fix it by using the errno on the error path.

Assisted-by: LLM (exposing the bug)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/image-restore.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/image/image-restore.c b/image/image-restore.c
index 7f3c9fdb6536..3d76440599ec 100644
--- a/image/image-restore.c
+++ b/image/image-restore.c
@@ -1889,6 +1889,7 @@ int restore_metadump(const char *input, FILE *out, int old_restore,
 	}
 
 	if (in != stdin && fseek(in, 0, SEEK_SET)) {
+		ret = -errno;
 		error("seek failed: %m");
 		goto out;
 	}
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-10-25  8:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25  8:41 [PATCH 00/12] btrfs-progs: image: various LLM found memory Qu Wenruo
2026-10-25  8:41 ` [PATCH 01/12] btrfs-progs: image: destroy chunk tree after all pthreads finished Qu Wenruo
2026-10-25  8:41 ` [PATCH 02/12] btrfs-progs: image: only remove a chunk map from physical tree if non-empty Qu Wenruo
2026-10-25  8:41 ` [PATCH 03/12] btrfs-progs: image: cleanup pending ordered works after an error Qu Wenruo
2026-10-25  8:41 ` [PATCH 04/12] btrfs-progs: image: do not cleanup the thread that failed to be created Qu Wenruo
2026-10-25  8:41 ` [PATCH 05/12] btrfs-progs: image: free the buffer if output allocation failed Qu Wenruo
2026-10-25  8:41 ` [PATCH 06/12] btrfs-progs: image: avoid NULL-ptr-deref in build_chunk_tree() Qu Wenruo
2026-10-25  8:41 ` [PATCH 07/12] btrfs-progs: image: fix the error handling of build_chunk_tree() Qu Wenruo
2026-10-25  8:41 ` [PATCH 08/12] btrfs-progs: image: fix memory leak on async_work for restore Qu Wenruo
2026-10-25  8:41 ` [PATCH 09/12] btrfs-progs: image: fix overwritten ret in restore_metadump() Qu Wenruo
2026-10-25  8:41 ` [PATCH 10/12] btrfs-progs: image: validate the nritems before reading the content Qu Wenruo
2026-10-25  8:41 ` [PATCH 11/12] btrfs-progs: image: reject incorrect super block size Qu Wenruo
2026-10-25  8:41 ` [PATCH 12/12] btrfs-progs: image: properly error out when fseek() failed Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox