mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] btrfs: verity: return error from btrfs_orphan_add in begin_enable_verity
@ 2026-10-15  7:11 guolingxing
  2026-10-15  9:02 ` Qu Wenruo
  0 siblings, 1 reply; 4+ messages in thread
From: guolingxing @ 2026-10-15  7:11 UTC (permalink / raw)
  To: David Sterba, Chris Mason, Boris Burkov
  Cc: linux-btrfs, linux-kernel, guolingxing, stable

btrfs_begin_enable_verity() ignores the return value of
btrfs_orphan_add() and unconditionally returns 0. When inserting the
orphan item fails (e.g. -ENOMEM), the error is swallowed and the
caller continues to build the Merkle tree even though the orphan item
was never written and BTRFS_INODE_VERITY_IN_PROGRESS was not set,
leaving the on-disk state inconsistent.

Propagate the error to the caller so that fsverity_ioctl_enable()
aborts before building the Merkle tree. The transaction is already
aborted inside btrfs_orphan_add() on failure and released by the
unconditional btrfs_end_transaction(), so only the return value needs
to change.

Fixes: 705242538ff3 ("btrfs: verity metadata orphan items")
Cc: stable@vger.kernel.org
Signed-off-by: guolingxing <guolingxing@tju.edu.cn>
---
 fs/btrfs/verity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 600337a84fbe..d92c9dab0293 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -615,7 +615,7 @@ static int btrfs_begin_enable_verity(struct file *filp)
 		set_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
 	btrfs_end_transaction(trans);
 
-	return 0;
+	return ret;
 }
 
 /*
-- 
2.34.1


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

* Re: [PATCH] btrfs: verity: return error from btrfs_orphan_add in begin_enable_verity
  2026-10-15  7:11 [PATCH] btrfs: verity: return error from btrfs_orphan_add in begin_enable_verity guolingxing
@ 2026-10-15  9:02 ` Qu Wenruo
  2026-10-15  9:59   ` [PATCH v2] btrfs: verity: propagate orphan insertion errors when enabling verity guolingxing
  0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2026-10-15  9:02 UTC (permalink / raw)
  To: guolingxing, David Sterba, Chris Mason, Boris Burkov
  Cc: linux-btrfs, linux-kernel, stable



在 2026/10/15 17:41, guolingxing 写道:
> btrfs_begin_enable_verity() ignores the return value of
> btrfs_orphan_add() and unconditionally returns 0. When inserting the
> orphan item fails (e.g. -ENOMEM), the error is swallowed and the
> caller continues to build the Merkle tree even though the orphan item
> was never written and BTRFS_INODE_VERITY_IN_PROGRESS was not set,
> leaving the on-disk state inconsistent.

This conflicts with your statement just lines below:

"The transaction is already aborted inside btrfs_orphan_add() on failure"

So why the on-disk state can still be inconsistent?

> 
> Propagate the error to the caller so that fsverity_ioctl_enable()
> aborts before building the Merkle tree. The transaction is already
> aborted inside btrfs_orphan_add() on failure and released by the
> unconditional btrfs_end_transaction(), so only the return value needs
> to change.
> 
> Fixes: 705242538ff3 ("btrfs: verity metadata orphan items")
> Cc: stable@vger.kernel.org
> Signed-off-by: guolingxing <guolingxing@tju.edu.cn>
> ---
>   fs/btrfs/verity.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 600337a84fbe..d92c9dab0293 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -615,7 +615,7 @@ static int btrfs_begin_enable_verity(struct file *filp)
>   		set_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
>   	btrfs_end_transaction(trans);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   /*


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

* [PATCH v2] btrfs: verity: propagate orphan insertion errors when enabling verity
  2026-10-15  9:02 ` Qu Wenruo
@ 2026-10-15  9:59   ` guolingxing
  2026-10-15 10:34     ` Qu Wenruo
  0 siblings, 1 reply; 4+ messages in thread
From: guolingxing @ 2026-10-15  9:59 UTC (permalink / raw)
  To: Qu Wenruo, David Sterba, Chris Mason, Boris Burkov
  Cc: linux-btrfs, linux-kernel, guolingxing

btrfs_begin_enable_verity() ignores the return value of
btrfs_orphan_add() and unconditionally returns 0. If orphan insertion
fails, btrfs_orphan_add() aborts the transaction, but the generic
fs-verity code still proceeds with Merkle tree construction because
the begin_enable_verity callback reported success.

Return the error so that fs-verity stops immediately instead of
continuing after the initialization failure.

The transaction is already aborted by btrfs_orphan_add() and the
handle is released by btrfs_end_transaction(), so no additional
cleanup is needed.

Fixes: 705242538ff3 ("btrfs: verity metadata orphan items")
Signed-off-by: guolingxing <guolingxing@tju.edu.cn>
---
Hi Qu, you are right. The claim about leaving the on-disk state
inconsistent was incorrect.

btrfs_orphan_add() aborts the transaction on failure, so I do not
have evidence that ignoring its return value can cause on-disk
inconsistency. The real issue is that btrfs_begin_enable_verity()
reports success despite the initialization failure, letting the
generic fs-verity code proceed with Merkle tree construction only to
encounter an error later, instead of returning the original error
immediately. This revision removes the on-disk inconsistency claim and
describes the error propagation issue instead.

Thanks for pointing this out.

Changes in v2:
- Remove the incorrect claim about on-disk inconsistency.
- Clarify that the fix propagates the initialization error immediately.

 fs/btrfs/verity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 600337a84fbe..d92c9dab0293 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -615,7 +615,7 @@ static int btrfs_begin_enable_verity(struct file *filp)
 		set_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
 	btrfs_end_transaction(trans);
 
-	return 0;
+	return ret;
 }
 
 /*
-- 
2.34.1


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

* Re: [PATCH v2] btrfs: verity: propagate orphan insertion errors when enabling verity
  2026-10-15  9:59   ` [PATCH v2] btrfs: verity: propagate orphan insertion errors when enabling verity guolingxing
@ 2026-10-15 10:34     ` Qu Wenruo
  0 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-10-15 10:34 UTC (permalink / raw)
  To: guolingxing, David Sterba, Chris Mason, Boris Burkov
  Cc: linux-btrfs, linux-kernel



在 2026/10/15 20:29, guolingxing 写道:
> btrfs_begin_enable_verity() ignores the return value of
> btrfs_orphan_add() and unconditionally returns 0. If orphan insertion
> fails, btrfs_orphan_add() aborts the transaction, but the generic
> fs-verity code still proceeds with Merkle tree construction because
> the begin_enable_verity callback reported success.
> 
> Return the error so that fs-verity stops immediately instead of
> continuing after the initialization failure.
> 
> The transaction is already aborted by btrfs_orphan_add() and the
> handle is released by btrfs_end_transaction(), so no additional
> cleanup is needed.
> 
> Fixes: 705242538ff3 ("btrfs: verity metadata orphan items")
> Signed-off-by: guolingxing <guolingxing@tju.edu.cn>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Will push to for-next branch soon.

> ---
> Hi Qu, you are right. The claim about leaving the on-disk state
> inconsistent was incorrect.
> 
> btrfs_orphan_add() aborts the transaction on failure, so I do not
> have evidence that ignoring its return value can cause on-disk
> inconsistency. The real issue is that btrfs_begin_enable_verity()
> reports success despite the initialization failure, letting the
> generic fs-verity code proceed with Merkle tree construction only to
> encounter an error later, instead of returning the original error
> immediately. This revision removes the on-disk inconsistency claim and
> describes the error propagation issue instead.
> 
> Thanks for pointing this out.
> 
> Changes in v2:
> - Remove the incorrect claim about on-disk inconsistency.
> - Clarify that the fix propagates the initialization error immediately.
> 
>   fs/btrfs/verity.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 600337a84fbe..d92c9dab0293 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -615,7 +615,7 @@ static int btrfs_begin_enable_verity(struct file *filp)
>   		set_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
>   	btrfs_end_transaction(trans);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   /*


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

end of thread, other threads:[~2026-10-15 10:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-15  7:11 [PATCH] btrfs: verity: return error from btrfs_orphan_add in begin_enable_verity guolingxing
2026-10-15  9:02 ` Qu Wenruo
2026-10-15  9:59   ` [PATCH v2] btrfs: verity: propagate orphan insertion errors when enabling verity guolingxing
2026-10-15 10:34     ` Qu Wenruo

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

all inboxes | Powered by JetHome®