public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] btrfs: fixes for the received subvol ioctl
@ 2026-02-27  0:11 fdmanana
  2026-02-27  0:11 ` [PATCH 1/3] btrfs: fix transaction abort on set received ioctl due to item overflow fdmanana
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: fdmanana @ 2026-02-27  0:11 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

Fix a bug that can be exploited by malicious users to trigger a transaction
abort and turn the filesystem to RO mode by assigning the same received UUID
to a bunch of subvolumes, plus a missing transaction abort if we fail to
update a root item, and a cleanup.

Filipe Manana (3):
  btrfs: fix transaction abort on set received ioctl due to item overflow
  btrfs: abort transaction on failure to update root in the received subvol ioctl
  btrfs: remove unnecessary transaction abort in the received subvol ioctl

 fs/btrfs/ioctl.c     | 23 ++++++++++++++++++++---
 fs/btrfs/uuid-tree.c | 38 ++++++++++++++++++++++++++++++++++++++
 fs/btrfs/uuid-tree.h |  2 ++
 3 files changed, 60 insertions(+), 3 deletions(-)

-- 
2.47.2


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

* [PATCH 1/3] btrfs: fix transaction abort on set received ioctl due to item overflow
  2026-02-27  0:11 [PATCH 0/3] btrfs: fixes for the received subvol ioctl fdmanana
@ 2026-02-27  0:11 ` fdmanana
  2026-02-27  0:11 ` [PATCH 2/3] btrfs: abort transaction on failure to update root in the received subvol ioctl fdmanana
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: fdmanana @ 2026-02-27  0:11 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

If the set received ioctl fails due to an item overflow when attempting to
add the BTRFS_UUID_KEY_RECEIVED_SUBVOL we have to abort the transaction
since we did some metadata updates before.

This means that if a user calls this ioctl with the same received UUID
field for a lot of subvolumes, we will hit the overflow, trigger the
transaction abort and turn the filesystem into RO mode. A malicious user
could exploit this, and this ioctl does not even requires that a user
has admin privileges (CAP_SYS_ADMIN), only that he/she owns the subvolume.

Fix this by doing an early check for item overflow before starting a
transaction. This is also race safe because we are holding the subvol_sem
semaphore in exclusive (write) mode.

A test case for fstests will follow soon.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ioctl.c     | 21 +++++++++++++++++++--
 fs/btrfs/uuid-tree.c | 38 ++++++++++++++++++++++++++++++++++++++
 fs/btrfs/uuid-tree.h |  2 ++
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index fa68fbeb6722..dd411b0732a7 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3865,6 +3865,25 @@ static long _btrfs_ioctl_set_received_subvol(struct file *file,
 		goto out;
 	}
 
+	received_uuid_changed = memcmp(root_item->received_uuid, sa->uuid,
+				       BTRFS_UUID_SIZE);
+
+	/*
+	 * Before we attempt to add the new received uuid, check if we have room
+	 * for it in case there's already an item. If the size of the existing
+	 * item plus this root's ID (u64) exceeds the maximum item size, we can
+	 * return here without the need to abort a transaction. If we don't do
+	 * this check, the btrfs_uuid_tree_add() call below would fail with
+	 * -EOVERFLOW and result in a transaction abort. Malicious users could
+	 * exploit this to turn the fs into RO mode.
+	 */
+	if (received_uuid_changed && !btrfs_is_empty_uuid(sa->uuid)) {
+		ret = btrfs_uuid_tree_check_overflow(fs_info, sa->uuid,
+						     BTRFS_UUID_KEY_RECEIVED_SUBVOL);
+		if (ret < 0)
+			goto out;
+	}
+
 	/*
 	 * 1 - root item
 	 * 2 - uuid items (received uuid + subvol uuid)
@@ -3880,8 +3899,6 @@ static long _btrfs_ioctl_set_received_subvol(struct file *file,
 	sa->rtime.sec = ct.tv_sec;
 	sa->rtime.nsec = ct.tv_nsec;
 
-	received_uuid_changed = memcmp(root_item->received_uuid, sa->uuid,
-				       BTRFS_UUID_SIZE);
 	if (received_uuid_changed &&
 	    !btrfs_is_empty_uuid(root_item->received_uuid)) {
 		ret = btrfs_uuid_tree_remove(trans, root_item->received_uuid,
diff --git a/fs/btrfs/uuid-tree.c b/fs/btrfs/uuid-tree.c
index 7942d3887515..276f0eb874d4 100644
--- a/fs/btrfs/uuid-tree.c
+++ b/fs/btrfs/uuid-tree.c
@@ -196,6 +196,44 @@ int btrfs_uuid_tree_remove(struct btrfs_trans_handle *trans, const u8 *uuid, u8
 	return 0;
 }
 
+/*
+ * Check if we can add one root ID to a UUID key.
+ * If the key does not yet exists, we can, otherwise only if extended item does
+ * not exceeds the maximum item size permitted by the leaf size.
+ *
+ * Returns 0 on success, negative value on error.
+ */
+int btrfs_uuid_tree_check_overflow(struct btrfs_fs_info *fs_info,
+				   const u8 *uuid, u8 type)
+{
+	BTRFS_PATH_AUTO_FREE(path);
+	int ret;
+	u32 item_size;
+	struct btrfs_key key;
+
+	if (WARN_ON_ONCE(!fs_info->uuid_root))
+		return -EINVAL;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	btrfs_uuid_to_key(uuid, type, &key);
+	ret = btrfs_search_slot(NULL, fs_info->uuid_root, &key, path, 0, 0);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		return 0;
+
+	item_size = btrfs_item_size(path->nodes[0], path->slots[0]);
+
+	if (sizeof(struct btrfs_item) + item_size + sizeof(u64) >
+	    BTRFS_LEAF_DATA_SIZE(fs_info))
+		return -EOVERFLOW;
+
+	return 0;
+}
+
 static int btrfs_uuid_iter_rem(struct btrfs_root *uuid_root, u8 *uuid, u8 type,
 			       u64 subid)
 {
diff --git a/fs/btrfs/uuid-tree.h b/fs/btrfs/uuid-tree.h
index c60ad20325cc..02b235a3653f 100644
--- a/fs/btrfs/uuid-tree.h
+++ b/fs/btrfs/uuid-tree.h
@@ -12,6 +12,8 @@ int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, const u8 *uuid, u8 typ
 			u64 subid);
 int btrfs_uuid_tree_remove(struct btrfs_trans_handle *trans, const u8 *uuid, u8 type,
 			u64 subid);
+int btrfs_uuid_tree_check_overflow(struct btrfs_fs_info *fs_info,
+				   const u8 *uuid, u8 type);
 int btrfs_uuid_tree_iterate(struct btrfs_fs_info *fs_info);
 int btrfs_create_uuid_tree(struct btrfs_fs_info *fs_info);
 int btrfs_uuid_scan_kthread(void *data);
-- 
2.47.2


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

* [PATCH 2/3] btrfs: abort transaction on failure to update root in the received subvol ioctl
  2026-02-27  0:11 [PATCH 0/3] btrfs: fixes for the received subvol ioctl fdmanana
  2026-02-27  0:11 ` [PATCH 1/3] btrfs: fix transaction abort on set received ioctl due to item overflow fdmanana
@ 2026-02-27  0:11 ` fdmanana
  2026-02-27  0:11 ` [PATCH 3/3] btrfs: remove unnecessary transaction abort " fdmanana
  2026-02-27 23:27 ` [PATCH 0/3] btrfs: fixes for " Anand Jain
  3 siblings, 0 replies; 6+ messages in thread
From: fdmanana @ 2026-02-27  0:11 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

If we failed to update the root we don't abort the transaction, which is
wrong since we already used the transaction to remove an item from the
uuid tree.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ioctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index dd411b0732a7..8799eb82c4c3 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3921,6 +3921,7 @@ static long _btrfs_ioctl_set_received_subvol(struct file *file,
 	ret = btrfs_update_root(trans, fs_info->tree_root,
 				&root->root_key, &root->root_item);
 	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
 		btrfs_end_transaction(trans);
 		goto out;
 	}
-- 
2.47.2


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

* [PATCH 3/3] btrfs: remove unnecessary transaction abort in the received subvol ioctl
  2026-02-27  0:11 [PATCH 0/3] btrfs: fixes for the received subvol ioctl fdmanana
  2026-02-27  0:11 ` [PATCH 1/3] btrfs: fix transaction abort on set received ioctl due to item overflow fdmanana
  2026-02-27  0:11 ` [PATCH 2/3] btrfs: abort transaction on failure to update root in the received subvol ioctl fdmanana
@ 2026-02-27  0:11 ` fdmanana
  2026-02-27 23:27 ` [PATCH 0/3] btrfs: fixes for " Anand Jain
  3 siblings, 0 replies; 6+ messages in thread
From: fdmanana @ 2026-02-27  0:11 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

If we fail to remove an item from the uuid tree, we don't need to abort
the transaction since we have not done any change before. So remove that
transaction abort.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ioctl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 8799eb82c4c3..3f63769c89b0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3905,7 +3905,6 @@ static long _btrfs_ioctl_set_received_subvol(struct file *file,
 					  BTRFS_UUID_KEY_RECEIVED_SUBVOL,
 					  btrfs_root_id(root));
 		if (unlikely(ret && ret != -ENOENT)) {
-		        btrfs_abort_transaction(trans, ret);
 		        btrfs_end_transaction(trans);
 		        goto out;
 		}
-- 
2.47.2


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

* Re: [PATCH 0/3] btrfs: fixes for the received subvol ioctl
  2026-02-27  0:11 [PATCH 0/3] btrfs: fixes for the received subvol ioctl fdmanana
                   ` (2 preceding siblings ...)
  2026-02-27  0:11 ` [PATCH 3/3] btrfs: remove unnecessary transaction abort " fdmanana
@ 2026-02-27 23:27 ` Anand Jain
  2026-02-28 17:49   ` Filipe Manana
  3 siblings, 1 reply; 6+ messages in thread
From: Anand Jain @ 2026-02-27 23:27 UTC (permalink / raw)
  To: fdmanana, linux-btrfs



> Filipe Manana (3):
>   btrfs: fix transaction abort on set received ioctl due to item overflow
>   btrfs: abort transaction on failure to update root in the received subvol ioctl
>   btrfs: remove unnecessary transaction abort in the received subvol ioctl



Can this also be considered for LTS kernels?

Otherwise, this looks good to me.

Reviewed-by: Anand Jain [asj@kernel.org](mailto:asj@kernel.org)

Anand

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

* Re: [PATCH 0/3] btrfs: fixes for the received subvol ioctl
  2026-02-27 23:27 ` [PATCH 0/3] btrfs: fixes for " Anand Jain
@ 2026-02-28 17:49   ` Filipe Manana
  0 siblings, 0 replies; 6+ messages in thread
From: Filipe Manana @ 2026-02-28 17:49 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Fri, Feb 27, 2026 at 11:27 PM Anand Jain <anajain.sg@gmail.com> wrote:
>
>
>
> > Filipe Manana (3):
> >   btrfs: fix transaction abort on set received ioctl due to item overflow
> >   btrfs: abort transaction on failure to update root in the received subvol ioctl
> >   btrfs: remove unnecessary transaction abort in the received subvol ioctl
>
>
>
> Can this also be considered for LTS kernels?

I forgot to add a Fixes tag, which is:

Fixes: dd5f9615fc5c ("Btrfs: maintain subvolume items in the UUID tree")

This is both for the first and second patches. I'll add it before
pushing to for-next, thanks.

>
> Otherwise, this looks good to me.
>
> Reviewed-by: Anand Jain [asj@kernel.org](mailto:asj@kernel.org)
>
> Anand

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

end of thread, other threads:[~2026-02-28 17:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-27  0:11 [PATCH 0/3] btrfs: fixes for the received subvol ioctl fdmanana
2026-02-27  0:11 ` [PATCH 1/3] btrfs: fix transaction abort on set received ioctl due to item overflow fdmanana
2026-02-27  0:11 ` [PATCH 2/3] btrfs: abort transaction on failure to update root in the received subvol ioctl fdmanana
2026-02-27  0:11 ` [PATCH 3/3] btrfs: remove unnecessary transaction abort " fdmanana
2026-02-27 23:27 ` [PATCH 0/3] btrfs: fixes for " Anand Jain
2026-02-28 17:49   ` Filipe Manana

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