public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] btrfs: a couple bug fixes for log replay
@ 2025-06-03 21:19 fdmanana
  2025-06-03 21:19 ` [PATCH v2 1/2] btrfs: fix invalid inode pointer dereferences during " fdmanana
  2025-06-03 21:19 ` [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log fdmanana
  0 siblings, 2 replies; 5+ messages in thread
From: fdmanana @ 2025-06-03 21:19 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

Fix invalid inode pointer dereferences in error paths of log replay
and stop ignoring invalid extent types.

V2: Updated patch 1/2 to avoid NULL pointer dereference for the case
    where we find an unexpected/invalid extent type and added RB tag.
    Added patch 2/2.

Filipe Manana (2):
  btrfs: fix invalid inode pointer dereferences during log replay
  btrfs: don't silently ignore unexpected extent type when replaying log

 fs/btrfs/tree-log.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

-- 
2.47.2


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

* [PATCH v2 1/2] btrfs: fix invalid inode pointer dereferences during log replay
  2025-06-03 21:19 [PATCH v2 0/2] btrfs: a couple bug fixes for log replay fdmanana
@ 2025-06-03 21:19 ` fdmanana
  2025-06-03 21:19 ` [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log fdmanana
  1 sibling, 0 replies; 5+ messages in thread
From: fdmanana @ 2025-06-03 21:19 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

In a few places where we call read_one_inode(), if we get a NULL pointer
we end up jumping into an error path, or fallthrough in case of
__add_inode_ref(), where we then do something like this:

   iput(&inode->vfs_inode);

which results in an invalid inode pointer that triggers an invalid memory
access, resulting in a crash.

Fix this by making sure we don't do such dereferences.

Fixes: b4c50cbb01a1 ("btrfs: return a btrfs_inode from read_one_inode()")
CC: stable@vger.kernel.org # 6.15+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/tree-log.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 34ed9b2b1b83..c8dcc7d3f4b0 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -668,15 +668,12 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		extent_end = ALIGN(start + size,
 				   fs_info->sectorsize);
 	} else {
-		ret = 0;
-		goto out;
+		return 0;
 	}
 
 	inode = read_one_inode(root, key->objectid);
-	if (!inode) {
-		ret = -EIO;
-		goto out;
-	}
+	if (!inode)
+		return -EIO;
 
 	/*
 	 * first check to see if we already have this extent in the
@@ -961,7 +958,8 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
 	ret = unlink_inode_for_log_replay(trans, dir, inode, &name);
 out:
 	kfree(name.name);
-	iput(&inode->vfs_inode);
+	if (inode)
+		iput(&inode->vfs_inode);
 	return ret;
 }
 
@@ -1176,8 +1174,8 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 					ret = unlink_inode_for_log_replay(trans,
 							victim_parent,
 							inode, &victim_name);
+					iput(&victim_parent->vfs_inode);
 				}
-				iput(&victim_parent->vfs_inode);
 				kfree(victim_name.name);
 				if (ret)
 					return ret;
-- 
2.47.2


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

* [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log
  2025-06-03 21:19 [PATCH v2 0/2] btrfs: a couple bug fixes for log replay fdmanana
  2025-06-03 21:19 ` [PATCH v2 1/2] btrfs: fix invalid inode pointer dereferences during " fdmanana
@ 2025-06-03 21:19 ` fdmanana
  2025-06-03 22:22   ` Boris Burkov
  2025-06-04 12:24   ` David Sterba
  1 sibling, 2 replies; 5+ messages in thread
From: fdmanana @ 2025-06-03 21:19 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

If there's an unexpected (invalid) extent type, we just silently ignore
it. This means a corruption or some bug somewhere, so instead return
-EUCLEAN to the caller, making log replay fail, and print an error message
with relevant information.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index c8dcc7d3f4b0..3f5593fe1215 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -668,7 +668,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		extent_end = ALIGN(start + size,
 				   fs_info->sectorsize);
 	} else {
-		return 0;
+		btrfs_err(fs_info,
+		  "unexpected extent type=%d root=%llu inode=%llu offset=%llu",
+			  found_type, btrfs_root_id(root), key->objectid, key->offset);
+		return -EUCLEAN;
 	}
 
 	inode = read_one_inode(root, key->objectid);
-- 
2.47.2


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

* Re: [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log
  2025-06-03 21:19 ` [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log fdmanana
@ 2025-06-03 22:22   ` Boris Burkov
  2025-06-04 12:24   ` David Sterba
  1 sibling, 0 replies; 5+ messages in thread
From: Boris Burkov @ 2025-06-03 22:22 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs

On Tue, Jun 03, 2025 at 10:19:58PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If there's an unexpected (invalid) extent type, we just silently ignore
> it. This means a corruption or some bug somewhere, so instead return
> -EUCLEAN to the caller, making log replay fail, and print an error message
> with relevant information.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Boris Burkov <boris@bur.io>

> ---
>  fs/btrfs/tree-log.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index c8dcc7d3f4b0..3f5593fe1215 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -668,7 +668,10 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
>  		extent_end = ALIGN(start + size,
>  				   fs_info->sectorsize);
>  	} else {
> -		return 0;
> +		btrfs_err(fs_info,
> +		  "unexpected extent type=%d root=%llu inode=%llu offset=%llu",
> +			  found_type, btrfs_root_id(root), key->objectid, key->offset);
> +		return -EUCLEAN;
>  	}
>  
>  	inode = read_one_inode(root, key->objectid);
> -- 
> 2.47.2
> 

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

* Re: [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log
  2025-06-03 21:19 ` [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log fdmanana
  2025-06-03 22:22   ` Boris Burkov
@ 2025-06-04 12:24   ` David Sterba
  1 sibling, 0 replies; 5+ messages in thread
From: David Sterba @ 2025-06-04 12:24 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs

On Tue, Jun 03, 2025 at 10:19:58PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If there's an unexpected (invalid) extent type, we just silently ignore
> it. This means a corruption or some bug somewhere, so instead return
> -EUCLEAN to the caller, making log replay fail, and print an error message
> with relevant information.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>

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

end of thread, other threads:[~2025-06-04 12:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-03 21:19 [PATCH v2 0/2] btrfs: a couple bug fixes for log replay fdmanana
2025-06-03 21:19 ` [PATCH v2 1/2] btrfs: fix invalid inode pointer dereferences during " fdmanana
2025-06-03 21:19 ` [PATCH v2 2/2] btrfs: don't silently ignore unexpected extent type when replaying log fdmanana
2025-06-03 22:22   ` Boris Burkov
2025-06-04 12:24   ` David Sterba

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