diff mbox series

[1/2] ocfs2: Add a sanity check for corrupted file system.

Message ID 20250102124539.711325-1-sunjunchao2870@gmail.com (mailing list archive)
State New
Headers show
Series [1/2] ocfs2: Add a sanity check for corrupted file system. | expand

Commit Message

Julian Sun Jan. 2, 2025, 12:45 p.m. UTC
Hi,

Recently syzbot reported a use-after-free issue[1].

The root cause of the problem is that the journal
inode recorded in this file system image is corrupted.
The value of "di->id2.i_list.l_next_free_rec" is 8193,
which is greater than the value of "di->id2.i_list.l_count" (19).

To solve this problem, an additional check should be added
within ocfs2_get_clusters_nocache(). If the check fails, an error will
be returned and the file system will be set to read-only.

[1]: https://lore.kernel.org/all/67577778.050a0220.a30f1.01bc.GAE@google.com/T/

Reported-and-tested-by: syzbot+2313dda4dc4885c93578@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2313dda4dc4885c93578
Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ocfs2/extent_map.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Joseph Qi Jan. 3, 2025, 1:19 a.m. UTC | #1
Actually this is v2.

On 2025/1/2 20:45, Julian Sun wrote:
> Hi,
> 
> Recently syzbot reported a use-after-free issue[1].
> 
> The root cause of the problem is that the journal
> inode recorded in this file system image is corrupted.
> The value of "di->id2.i_list.l_next_free_rec" is 8193,
> which is greater than the value of "di->id2.i_list.l_count" (19).
> 
> To solve this problem, an additional check should be added
> within ocfs2_get_clusters_nocache(). If the check fails, an error will
> be returned and the file system will be set to read-only.
> 
> [1]: https://lore.kernel.org/all/67577778.050a0220.a30f1.01bc.GAE@google.com/T/
> 
> Reported-and-tested-by: syzbot+2313dda4dc4885c93578@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2313dda4dc4885c93578
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> ---
>  fs/ocfs2/extent_map.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index f7672472fa82..83d3343b3e3c 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -435,6 +435,15 @@ static int ocfs2_get_clusters_nocache(struct inode *inode,
>  		}
>  	}
>  
> +	if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
> +		ret = ocfs2_error(inode->i_sb, "Invalid dinode #%llu: next_free_rec is %u, count is %u\n",
> +				  (unsigned long long)di_bh->b_blocknr,
> +				  le16_to_cpu(di->id2.i_list.l_next_free_rec),
> +				  le16_to_cpu(di->id2.i_list.l_count));
> +
> +		goto out;
> +	}
> +
>  	i = ocfs2_search_extent_list(el, v_cluster);
>  	if (i == -1) {
>  		/*
Glass Su Jan. 3, 2025, 2:15 a.m. UTC | #2
The subject is too generic. Would you please change it to something like:

"ocfs2: check ocfs2_extent_list::l_next_free_rec in ocfs2_get_clusters_nocache”?

— 
Su

> On Jan 2, 2025, at 20:45, Julian Sun <sunjunchao2870@gmail.com> wrote:
> 
> Hi,
> 
> Recently syzbot reported a use-after-free issue[1].
> 
> The root cause of the problem is that the journal
> inode recorded in this file system image is corrupted.
> The value of "di->id2.i_list.l_next_free_rec" is 8193,
> which is greater than the value of "di->id2.i_list.l_count" (19).
> 
> To solve this problem, an additional check should be added
> within ocfs2_get_clusters_nocache(). If the check fails, an error will
> be returned and the file system will be set to read-only.
> 
> [1]: https://lore.kernel.org/all/67577778.050a0220.a30f1.01bc.GAE@google.com/T/
> 
> Reported-and-tested-by: syzbot+2313dda4dc4885c93578@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2313dda4dc4885c93578
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> ---
> fs/ocfs2/extent_map.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index f7672472fa82..83d3343b3e3c 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -435,6 +435,15 @@ static int ocfs2_get_clusters_nocache(struct inode *inode,
> }
> }
> 
> + if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
> + ret = ocfs2_error(inode->i_sb, "Invalid dinode #%llu: next_free_rec is %u, count is %u\n",
> +  (unsigned long long)di_bh->b_blocknr,
> +  le16_to_cpu(di->id2.i_list.l_next_free_rec),
> +  le16_to_cpu(di->id2.i_list.l_count));
> +
> + goto out;
> + }
> +
> i = ocfs2_search_extent_list(el, v_cluster);
> if (i == -1) {
> /*
> -- 
> 2.39.5
> 
>
Joseph Qi Jan. 3, 2025, 2:48 a.m. UTC | #3
On 2025/1/3 10:15, Glass Su wrote:
> The subject is too generic. Would you please change it to something like:
> 
> "ocfs2: check ocfs2_extent_list::l_next_free_rec in ocfs2_get_clusters_nocache”?
> 

Or simplify to
ocfs2: check el->l_next_free_rec in ocfs2_get_clusters_nocache
Julian Sun Jan. 3, 2025, 3:30 a.m. UTC | #4
Joseph Qi <joseph.qi@linux.alibaba.com> 于2025年1月3日周五 10:48写道:
>
>
>
> On 2025/1/3 10:15, Glass Su wrote:
> > The subject is too generic. Would you please change it to something like:
> >
> > "ocfs2: check ocfs2_extent_list::l_next_free_rec in ocfs2_get_clusters_nocache”?
> >
>
> Or simplify to
> ocfs2: check el->l_next_free_rec in ocfs2_get_clusters_nocache

Fixed it in next version, thanks.
diff mbox series

Patch

diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index f7672472fa82..83d3343b3e3c 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -435,6 +435,15 @@  static int ocfs2_get_clusters_nocache(struct inode *inode,
 		}
 	}
 
+	if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
+		ret = ocfs2_error(inode->i_sb, "Invalid dinode #%llu: next_free_rec is %u, count is %u\n",
+				  (unsigned long long)di_bh->b_blocknr,
+				  le16_to_cpu(di->id2.i_list.l_next_free_rec),
+				  le16_to_cpu(di->id2.i_list.l_count));
+
+		goto out;
+	}
+
 	i = ocfs2_search_extent_list(el, v_cluster);
 	if (i == -1) {
 		/*