[PATCH v4] ocfs2: validate la_size before ocfs2_clear_local_alloc()

[PATCH v4] ocfs2: validate la_size before ocfs2_clear_local_alloc()

[Dmitry Morgun]

la_size, like the journal dirty flag, is read from disk. If dirty != 0,
ocfs2_begin_local_alloc_recovery() is called, which invokes
ocfs2_clear_local_alloc(). At this point, ocfs2_clear_local_alloc()
uses la_size as the loop bound without validating it first. If
la_size is oversized, the loop writes past the end of la_bitmap and
may eventually start writing into memory that has already been freed.

BUG: KASAN: use-after-free in ocfs2_clear_local_alloc fs/ocfs2/localalloc.c:919 [inline]
BUG: KASAN: use-after-free in ocfs2_begin_local_alloc_recovery+0xb07/0xc00 fs/ocfs2/localalloc.c:515
CPU: 1 PID: 2386 Comm: syz.2.190 Not tainted 6.1.174-syzkaller-00520-g10c505401422 #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
Call Trace:
 <TASK>
 ocfs2_clear_local_alloc fs/ocfs2/localalloc.c:919 [inline]
 ocfs2_begin_local_alloc_recovery+0xb07/0xc00 fs/ocfs2/localalloc.c:515
 ocfs2_check_volume fs/ocfs2/super.c:2448 [inline]
 ocfs2_mount_volume fs/ocfs2/super.c:1819 [inline]
 ocfs2_fill_super+0x2033/0x3dc0 fs/ocfs2/super.c:1082
 mount_bdev+0x356/0x410 fs/super.c:1443
 legacy_get_tree+0x108/0x220 fs/fs_context.c:632
 vfs_get_tree+0x8e/0x300 fs/super.c:1573
 do_new_mount fs/namespace.c:3078 [inline]
 path_mount+0x6af/0x1f70 fs/namespace.c:3408
 do_mount fs/namespace.c:3421 [inline]
 __do_sys_mount fs/namespace.c:3629 [inline]
 __se_sys_mount fs/namespace.c:3606 [inline]
 __x64_sys_mount+0x283/0x300 fs/namespace.c:3606
 do_syscall_x64 arch/x86/entry/common.c:46 [inline]
 do_syscall_64+0x35/0x80 fs/namespace.c:76
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8

The existing la_size check in ocfs2_load_local_alloc() runs after
ocfs2_clear_local_alloc() on this recovery path. Memory corruption
can therefore occur before the invalid size is detected and
-EINVAL is returned.

Reject inodes without OCFS2_LOCAL_ALLOC_FL in
ocfs2_begin_local_alloc_recovery() before copying or clearing the
bitmap, returning -EINVAL. The recovery code interprets id2 as
i_lab, so it must not proceed when the local allocation flag is
missing.

Add la_size validation to ocfs2_validate_inode_block() for inodes
marked with OCFS2_LOCAL_ALLOC_FL. Reject zero or a size exceeding
the bitmap capacity through ocfs2_error(), which marks the
filesystem read-only and returns -EROFS. The flag check in recovery
prevents a corrupted inode from bypassing this validation by
clearing OCFS2_LOCAL_ALLOC_FL.

Keep the existing size check in ocfs2_load_local_alloc(), whose
flag check also accepts OCFS2_BITMAP_FL without OCFS2_LOCAL_ALLOC_FL.

The new checks can also return errors during node recovery. The
recovery thread leaves a failed node in the recovery map and
immediately retries it, so a persistent error prevents the loop
from completing. Release the super lock and exit through the
worker error path on failure, preserving the error status and
leaving the failed node in the recovery map.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Morgun <d.morgun@ispras.ru>
---
v4:
- Supersede v3, which was an unintended resend of the v1-based
  implementation.
- Move la_size validation from
  ocfs2_begin_local_alloc_recovery() to
  ocfs2_validate_inode_block() (Joseph Qi).
- Reject local allocation recovery when OCFS2_LOCAL_ALLOC_FL is
  missing, before copying or clearing the bitmap (Heming Zhao).
- Keep the existing size check in ocfs2_load_local_alloc().
- Release the super lock and exit the recovery thread on error,
  preserving the error status and the failed recovery-map entry
  instead of immediately retrying the same node.

Link to v3:
https://lore.kernel.org/ocfs2-devel/20260831143734.7243-1-d.morgun@ispras.ru/

v3:
- Unintended resend of the v1-based implementation.

v2:
- Not posted publicly.

Link to v1:
https://lore.kernel.org/all/20260810113508.8513-1-d.morgun@ispras.ru/

 fs/ocfs2/inode.c      | 13 +++++++++++++
 fs/ocfs2/journal.c    |  2 ++
 fs/ocfs2/localalloc.c |  9 ++++++++-
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index 9a6b1af4c..cc2233268 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1477,6 +1477,19 @@ int ocfs2_validate_inode_block(struct super_block *sb,
 		}
 	}
 
+	if (le32_to_cpu(di->i_flags) & OCFS2_LOCAL_ALLOC_FL) {
+		struct ocfs2_local_alloc *la = &di->id2.i_lab;
+
+		if (la->la_size == 0 ||
+		    le16_to_cpu(la->la_size) > ocfs2_local_alloc_size(sb)) {
+			rc = ocfs2_error(sb,
+					 "Invalid dinode #%llu: invalid la_size %u\n",
+					 (unsigned long long)bh->b_blocknr,
+					 le16_to_cpu(la->la_size));
+			goto bail;
+		}
+	}
+
 	rc = 0;
 
 bail:
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 7f5fd16d3..46983a061 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1525,6 +1525,8 @@ static int __ocfs2_recovery_thread(void *arg)
 			     status, node_num,
 			     MAJOR(osb->sb->s_dev), MINOR(osb->sb->s_dev));
 			mlog(ML_ERROR, "Volume requires unmount.\n");
+			ocfs2_super_unlock(osb, 1);
+			goto bail;
 		}
 
 		spin_lock(&osb->osb_lock);
diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index c4426d12a..c5c80a17b 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -504,6 +504,14 @@ int ocfs2_begin_local_alloc_recovery(struct ocfs2_super *osb,
 		goto bail;
 	}
 
+	alloc = (struct ocfs2_dinode *)alloc_bh->b_data;
+	if (!(le32_to_cpu(alloc->i_flags) & OCFS2_LOCAL_ALLOC_FL)) {
+		mlog(ML_ERROR, "Invalid local alloc inode, %llu\n",
+		     (unsigned long long)OCFS2_I(inode)->ip_blkno);
+		status = -EINVAL;
+		goto bail;
+	}
+
 	*alloc_copy = kmalloc(alloc_bh->b_size, GFP_KERNEL);
 	if (!(*alloc_copy)) {
 		status = -ENOMEM;
@@ -511,7 +519,6 @@ int ocfs2_begin_local_alloc_recovery(struct ocfs2_super *osb,
 	}
 	memcpy((*alloc_copy), alloc_bh->b_data, alloc_bh->b_size);
 
-	alloc = (struct ocfs2_dinode *) alloc_bh->b_data;
 	ocfs2_clear_local_alloc(alloc);
 
 	ocfs2_compute_meta_ecc(osb->sb, alloc_bh->b_data, &alloc->i_check);
-- 
2.34.1

Re: [PATCH v4] ocfs2: validate la_size before ocfs2_clear_local_alloc()

[Joseph Qi]

On 10/6/26 8:28 PM, Dmitry Morgun wrote:
> la_size, like the journal dirty flag, is read from disk. If dirty != 0,
> ocfs2_begin_local_alloc_recovery() is called, which invokes
> ocfs2_clear_local_alloc(). At this point, ocfs2_clear_local_alloc()
> uses la_size as the loop bound without validating it first. If
> la_size is oversized, the loop writes past the end of la_bitmap and
> may eventually start writing into memory that has already been freed.
> 
> BUG: KASAN: use-after-free in ocfs2_clear_local_alloc fs/ocfs2/localalloc.c:919 [inline]
> BUG: KASAN: use-after-free in ocfs2_begin_local_alloc_recovery+0xb07/0xc00 fs/ocfs2/localalloc.c:515
> CPU: 1 PID: 2386 Comm: syz.2.190 Not tainted 6.1.174-syzkaller-00520-g10c505401422 #0
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> Call Trace:
>  <TASK>
>  ocfs2_clear_local_alloc fs/ocfs2/localalloc.c:919 [inline]
>  ocfs2_begin_local_alloc_recovery+0xb07/0xc00 fs/ocfs2/localalloc.c:515
>  ocfs2_check_volume fs/ocfs2/super.c:2448 [inline]
>  ocfs2_mount_volume fs/ocfs2/super.c:1819 [inline]
>  ocfs2_fill_super+0x2033/0x3dc0 fs/ocfs2/super.c:1082
>  mount_bdev+0x356/0x410 fs/super.c:1443
>  legacy_get_tree+0x108/0x220 fs/fs_context.c:632
>  vfs_get_tree+0x8e/0x300 fs/super.c:1573
>  do_new_mount fs/namespace.c:3078 [inline]
>  path_mount+0x6af/0x1f70 fs/namespace.c:3408
>  do_mount fs/namespace.c:3421 [inline]
>  __do_sys_mount fs/namespace.c:3629 [inline]
>  __se_sys_mount fs/namespace.c:3606 [inline]
>  __x64_sys_mount+0x283/0x300 fs/namespace.c:3606
>  do_syscall_x64 arch/x86/entry/common.c:46 [inline]
>  do_syscall_64+0x35/0x80 fs/namespace.c:76
>  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> 
> The existing la_size check in ocfs2_load_local_alloc() runs after
> ocfs2_clear_local_alloc() on this recovery path. Memory corruption
> can therefore occur before the invalid size is detected and
> -EINVAL is returned.
> 
> Reject inodes without OCFS2_LOCAL_ALLOC_FL in
> ocfs2_begin_local_alloc_recovery() before copying or clearing the
> bitmap, returning -EINVAL. The recovery code interprets id2 as
> i_lab, so it must not proceed when the local allocation flag is
> missing.
> 
> Add la_size validation to ocfs2_validate_inode_block() for inodes
> marked with OCFS2_LOCAL_ALLOC_FL. Reject zero or a size exceeding
> the bitmap capacity through ocfs2_error(), which marks the
> filesystem read-only and returns -EROFS. The flag check in recovery

The behavior depends on errors= settings, so -EROFS is not always true. 

> prevents a corrupted inode from bypassing this validation by
> clearing OCFS2_LOCAL_ALLOC_FL.
> 
> Keep the existing size check in ocfs2_load_local_alloc(), whose
> flag check also accepts OCFS2_BITMAP_FL without OCFS2_LOCAL_ALLOC_FL.
> 
> The new checks can also return errors during node recovery. The
> recovery thread leaves a failed node in the recovery map and
> immediately retries it, so a persistent error prevents the loop
> from completing. Release the super lock and exit through the
> worker error path on failure, preserving the error status and
> leaving the failed node in the recovery map.
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Morgun <d.morgun@ispras.ru>
> ---
> v4:
> - Supersede v3, which was an unintended resend of the v1-based
>   implementation.
> - Move la_size validation from
>   ocfs2_begin_local_alloc_recovery() to
>   ocfs2_validate_inode_block() (Joseph Qi).
> - Reject local allocation recovery when OCFS2_LOCAL_ALLOC_FL is
>   missing, before copying or clearing the bitmap (Heming Zhao).
> - Keep the existing size check in ocfs2_load_local_alloc().
> - Release the super lock and exit the recovery thread on error,
>   preserving the error status and the failed recovery-map entry
>   instead of immediately retrying the same node.
> 
> Link to v3:
> https://lore.kernel.org/ocfs2-devel/20260831143734.7243-1-d.morgun@ispras.ru/
> 
> v3:
> - Unintended resend of the v1-based implementation.
> 
> v2:
> - Not posted publicly.
> 
> Link to v1:
> https://lore.kernel.org/all/20260810113508.8513-1-d.morgun@ispras.ru/
> 
>  fs/ocfs2/inode.c      | 13 +++++++++++++
>  fs/ocfs2/journal.c    |  2 ++
>  fs/ocfs2/localalloc.c |  9 ++++++++-
>  3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index 9a6b1af4c..cc2233268 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1477,6 +1477,19 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  		}
>  	}
>  
> +	if (le32_to_cpu(di->i_flags) & OCFS2_LOCAL_ALLOC_FL) {
> +		struct ocfs2_local_alloc *la = &di->id2.i_lab;
> +
> +		if (la->la_size == 0 ||
> +		    le16_to_cpu(la->la_size) > ocfs2_local_alloc_size(sb)) {
> +			rc = ocfs2_error(sb,
> +					 "Invalid dinode #%llu: invalid la_size %u\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 le16_to_cpu(la->la_size));
> +			goto bail;
> +		}
> +	}
> +
>  	rc = 0;
>  
>  bail:
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 7f5fd16d3..46983a061 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -1525,6 +1525,8 @@ static int __ocfs2_recovery_thread(void *arg)
>  			     status, node_num,
>  			     MAJOR(osb->sb->s_dev), MINOR(osb->sb->s_dev));
>  			mlog(ML_ERROR, "Volume requires unmount.\n");
> +			ocfs2_super_unlock(osb, 1);
> +			goto bail;

This fires on any non-zero status from ocfs2_recover_node(), not only on
persistent ones, e.g. -ENOMEM returns from
ocfs2_begin_local_alloc_recovery().

Thanks,
Joseph

>  		}
>  
>  		spin_lock(&osb->osb_lock);
> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
> index c4426d12a..c5c80a17b 100644
> --- a/fs/ocfs2/localalloc.c
> +++ b/fs/ocfs2/localalloc.c
> @@ -504,6 +504,14 @@ int ocfs2_begin_local_alloc_recovery(struct ocfs2_super *osb,
>  		goto bail;
>  	}
>  
> +	alloc = (struct ocfs2_dinode *)alloc_bh->b_data;
> +	if (!(le32_to_cpu(alloc->i_flags) & OCFS2_LOCAL_ALLOC_FL)) {
> +		mlog(ML_ERROR, "Invalid local alloc inode, %llu\n",
> +		     (unsigned long long)OCFS2_I(inode)->ip_blkno);
> +		status = -EINVAL;
> +		goto bail;
> +	}
> +
>  	*alloc_copy = kmalloc(alloc_bh->b_size, GFP_KERNEL);
>  	if (!(*alloc_copy)) {
>  		status = -ENOMEM;
> @@ -511,7 +519,6 @@ int ocfs2_begin_local_alloc_recovery(struct ocfs2_super *osb,
>  	}
>  	memcpy((*alloc_copy), alloc_bh->b_data, alloc_bh->b_size);
>  
> -	alloc = (struct ocfs2_dinode *) alloc_bh->b_data;
>  	ocfs2_clear_local_alloc(alloc);
>  
>  	ocfs2_compute_meta_ecc(osb->sb, alloc_bh->b_data, &alloc->i_check);