[PATCH v2 0/2] ocfs2: bound-check both readdir re-validation scans

[PATCH v2 0/2] ocfs2: bound-check both readdir re-validation scans

[Zhan Xusheng]

Andrew, patch 1 replaces
  ocfs2-bound-check-dir-entries-in-the-readdir-re-validation-scan.patch
in mm-nonmm-unstable.  Patch 2 is new.

Both are against that tree rather than mainline, because the patch queued
ahead of them rewrote the ctx->pos line that is now in patch 1's trailing
context.

v1 only bounded the position before de is dereferenced.  Joseph pointed out
that the walk has to stay inside the block as well, which is the second
guard ocfs2_check_dir_entry() applies for the emit loop; patch 1 adds it.
Checking whether anything else re-scans the same way turned up
ocfs2_dir_foreach_blk_id(), which walks the inline data area and is missing
both guards -- that is patch 2, which Joseph has not seen yet.

Neither patch carries Fixes: or Cc: stable, matching the trailers kept for
v1; please add them if you would rather they were there.

Link to v1: https://lore.kernel.org/all/20260806122133.956847-1-zhanxusheng@xiaomi.com/

Zhan Xusheng (2):
  ocfs2: bound-check dir entries in the readdir re-validation scan
  ocfs2: bound-check dir entries in the inline-data re-validation scan

 fs/ocfs2/dir.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] ocfs2: bound-check dir entries in the readdir re-validation scan

[Zhan Xusheng]

When the inode version changed since the last readdir(),
ocfs2_dir_foreach_blk_el() re-scans the directory block from its start to
relocate the current position:

	for (i = 0; i < sb->s_blocksize && i < offset; ) {
		de = (struct ocfs2_dir_entry *)(bh->b_data + i);
		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
			break;
		i += le16_to_cpu(de->rec_len);
	}

i walks the block on rec_len values taken from the block itself and the
only thing tested is that rec_len is not too small, so a single bogus
rec_len
leaves i anywhere in the block, including its last OCFS2_DIR_REC_LEN(1) - 1
bytes.  @offset comes from ctx->pos, which userspace moves with lseek() on
the directory fd, and decides how far the walk gets.

Two bounds are missing, both of which ocfs2_check_dir_entry() applies for
the emit loop below.

de->rec_len sits at byte offset 8 within the entry, so dereferencing de in
that tail reads past the s_blocksize buffer.  ocfs2_check_dir_entry()
declines to look at an entry that close to the end:

	size - buf_offset < OCFS2_DIR_REC_LEN(1)

Nothing bounds i += rec_len either, so i can end up past the block.  The
emit loop that follows is guarded by offset < sb->s_blocksize and does not
run, but

	offset = i;
	ctx->pos = (ctx->pos & ~((loff_t)sb->s_blocksize - 1)) | offset;

runs first and ORs a value with bits above the block mask into ctx->pos,
corrupting the block number readdir() resumes from.
ocfs2_check_dir_entry() rejects that as "directory entry overrun":

	next_offset = buf_offset + rlen;
	... next_offset > size

Apply both bounds.  For a consistent directory this changes nothing:
entries are at least OCFS2_DIR_REC_LEN(1) bytes and do not cross the end of
the block, so no valid entry is skipped.

Found by the sashiko review tool; fix approach suggested by Joseph Qi.

Link: https://sashiko.dev/#/patchset/20260806022044.167962-1-zhanxusheng@xiaomi.com
Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ocfs2/dir.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index c30a86856d5b..39ce60874e17 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1903,7 +1903,10 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
 		 * dirent right now.  Scan from the start of the block
 		 * to make sure. */
 		if (!inode_eq_iversion(inode, *f_version)) {
-			for (i = 0; i < sb->s_blocksize && i < offset; ) {
+			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= sb->s_blocksize &&
+			     i < offset;) {
+				unsigned int rec_len;
+
 				de = (struct ocfs2_dir_entry *) (bh->b_data + i);
 				/* It's too expensive to do a full
 				 * dirent test each time round this
@@ -1911,10 +1914,11 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
 				 * least that it is non-zero.  A
 				 * failure will be detected in the
 				 * dirent test below. */
-				if (le16_to_cpu(de->rec_len) <
-				    OCFS2_DIR_REC_LEN(1))
+				rec_len = le16_to_cpu(de->rec_len);
+				if (rec_len < OCFS2_DIR_REC_LEN(1) ||
+				    i + rec_len > sb->s_blocksize)
 					break;
-				i += le16_to_cpu(de->rec_len);
+				i += rec_len;
 			}
 			offset = i;
 			ctx->pos = (ctx->pos & ~((loff_t)sb->s_blocksize - 1))
-- 
2.43.0

Re: [PATCH v2 1/2] ocfs2: bound-check dir entries in the readdir re-validation scan

[Joseph Qi]

On 8/11/26 10:43 AM, Zhan Xusheng wrote:
> When the inode version changed since the last readdir(),
> ocfs2_dir_foreach_blk_el() re-scans the directory block from its start to
> relocate the current position:
> 
> 	for (i = 0; i < sb->s_blocksize && i < offset; ) {
> 		de = (struct ocfs2_dir_entry *)(bh->b_data + i);
> 		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
> 			break;
> 		i += le16_to_cpu(de->rec_len);
> 	}
> 
> i walks the block on rec_len values taken from the block itself and the
> only thing tested is that rec_len is not too small, so a single bogus
> rec_len
> leaves i anywhere in the block, including its last OCFS2_DIR_REC_LEN(1) - 1
> bytes.  @offset comes from ctx->pos, which userspace moves with lseek() on
> the directory fd, and decides how far the walk gets.
> 
> Two bounds are missing, both of which ocfs2_check_dir_entry() applies for
> the emit loop below.
> 
> de->rec_len sits at byte offset 8 within the entry, so dereferencing de in
> that tail reads past the s_blocksize buffer.  ocfs2_check_dir_entry()
> declines to look at an entry that close to the end:
> 
> 	size - buf_offset < OCFS2_DIR_REC_LEN(1)
> 
> Nothing bounds i += rec_len either, so i can end up past the block.  The
> emit loop that follows is guarded by offset < sb->s_blocksize and does not
> run, but
> 
> 	offset = i;
> 	ctx->pos = (ctx->pos & ~((loff_t)sb->s_blocksize - 1)) | offset;
> 
> runs first and ORs a value with bits above the block mask into ctx->pos,
> corrupting the block number readdir() resumes from.
> ocfs2_check_dir_entry() rejects that as "directory entry overrun":
> 
> 	next_offset = buf_offset + rlen;
> 	... next_offset > size
> 
> Apply both bounds.  For a consistent directory this changes nothing:
> entries are at least OCFS2_DIR_REC_LEN(1) bytes and do not cross the end of
> the block, so no valid entry is skipped.
> 
> Found by the sashiko review tool; fix approach suggested by Joseph Qi.
> 
> Link: https://sashiko.dev/#/patchset/20260806022044.167962-1-zhanxusheng@xiaomi.com
> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Cc: Mark Fasheh <mark@fasheh.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Cc: Changwei Ge <gechangwei@live.cn>
> Cc: Jun Piao <piaojun@huawei.com>
> Cc: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/dir.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index c30a86856d5b..39ce60874e17 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1903,7 +1903,10 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
>  		 * dirent right now.  Scan from the start of the block
>  		 * to make sure. */
>  		if (!inode_eq_iversion(inode, *f_version)) {
> -			for (i = 0; i < sb->s_blocksize && i < offset; ) {
> +			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= sb->s_blocksize &&
> +			     i < offset;) {
> +				unsigned int rec_len;
> +
>  				de = (struct ocfs2_dir_entry *) (bh->b_data + i);
>  				/* It's too expensive to do a full
>  				 * dirent test each time round this
> @@ -1911,10 +1914,11 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
>  				 * least that it is non-zero.  A
>  				 * failure will be detected in the
>  				 * dirent test below. */
> -				if (le16_to_cpu(de->rec_len) <
> -				    OCFS2_DIR_REC_LEN(1))
> +				rec_len = le16_to_cpu(de->rec_len);
> +				if (rec_len < OCFS2_DIR_REC_LEN(1) ||
> +				    i + rec_len > sb->s_blocksize)
>  					break;
> -				i += le16_to_cpu(de->rec_len);
> +				i += rec_len;
>  			}
>  			offset = i;
>  			ctx->pos = (ctx->pos & ~((loff_t)sb->s_blocksize - 1))

Re: [PATCH v2 1/2] ocfs2: bound-check dir entries in the readdir re-validation scan

[Andrew Morton]

On Tue, 11 Aug 2026 10:43:36 +0800 Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:

> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
> ...
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

checkpatch:

WARNING: From:/Signed-off-by: email address mismatch: 'From: Zhan Xusheng <zhanxusheng1024@gmail.com>' != 'Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>'


This can be prevented by using an explicit From: line at
start-of-changelog.  Please let me know which address should be used
and I'll make the edit, thanks.

Re: [PATCH v2 1/2] ocfs2: bound-check dir entries in the readdir re-validation scan

[Zhan Xusheng]

On Tue, 11 Aug 2026 19:22:31 -0700, Andrew Morton wrote:
> This can be prevented by using an explicit From: line at
> start-of-changelog.  Please let me know which address should be used
> and I'll make the edit, thanks.

zhanxusheng@xiaomi.com please, for both patches in the series.

The header was rewritten by the smarthost I send through, which is why it
disagreed with the Signed-off-by.  I have set format.from so the explicit
From: line is there from now on.

Thanks,
Zhan Xusheng

[PATCH v2 2/2] ocfs2: bound-check dir entries in the inline-data re-validation scan

[Zhan Xusheng]

ocfs2_dir_foreach_blk_id() re-scans the inline data area the same way
ocfs2_dir_foreach_blk_el() re-scans a directory block, and is missing the
same two bounds:

	for (i = 0; i < i_size_read(inode) && i < offset; ) {
		de = (struct ocfs2_dir_entry *)(data->id_data + i);
		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
			break;
		i += le16_to_cpu(de->rec_len);
	}

ocfs2_validate_inode_block() keeps i_size inside the inline area:

	if (le16_to_cpu(data->id_count) >
	    ocfs2_max_inline_data_with_xattr(sb, di))
	if (le64_to_cpu(di->i_size) > le16_to_cpu(data->id_count))

and that area runs to the end of the inode block, so for a full inline
directory data->id_data + i_size is the end of di_bh->b_data.  A bogus
rec_len leaves i in the last OCFS2_DIR_REC_LEN(1) - 1 bytes of it, and
de->rec_len, at byte offset 8 within the entry, is then read past the
block.

The emit loop below hands i_size_read(inode) to ocfs2_check_dir_entry(),
which refuses both an entry that close to the end and one whose rec_len
runs past it.  Apply the same two bounds to the re-validation scan, reading
i_size once into a local as ocfs2_check_dir_entry() takes it as @size.

Unlike the extent case there is no mask to corrupt here: an unbounded i
only sets ctx->pos past i_size, which ends the readdir early rather
than moving it to the wrong place.

Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ocfs2/dir.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index 39ce60874e17..863c69dec7da 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1812,7 +1812,12 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
 		 * dirent right now.  Scan from the start of the block
 		 * to make sure. */
 		if (!inode_eq_iversion(inode, *f_version)) {
-			for (i = 0; i < i_size_read(inode) && i < offset; ) {
+			loff_t size = i_size_read(inode);
+
+			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= size &&
+			     i < offset;) {
+				unsigned int rec_len;
+
 				de = (struct ocfs2_dir_entry *)
 					(data->id_data + i);
 				/* It's too expensive to do a full
@@ -1821,10 +1826,11 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
 				 * least that it is non-zero.  A
 				 * failure will be detected in the
 				 * dirent test below. */
-				if (le16_to_cpu(de->rec_len) <
-				    OCFS2_DIR_REC_LEN(1))
+				rec_len = le16_to_cpu(de->rec_len);
+				if (rec_len < OCFS2_DIR_REC_LEN(1) ||
+				    i + rec_len > size)
 					break;
-				i += le16_to_cpu(de->rec_len);
+				i += rec_len;
 			}
 			ctx->pos = offset = i;
 			*f_version = inode_query_iversion(inode);
-- 
2.43.0

Re: [PATCH v2 2/2] ocfs2: bound-check dir entries in the inline-data re-validation scan

[Joseph Qi]

On 8/11/26 10:43 AM, Zhan Xusheng wrote:
> ocfs2_dir_foreach_blk_id() re-scans the inline data area the same way
> ocfs2_dir_foreach_blk_el() re-scans a directory block, and is missing the
> same two bounds:
> 
> 	for (i = 0; i < i_size_read(inode) && i < offset; ) {
> 		de = (struct ocfs2_dir_entry *)(data->id_data + i);
> 		if (le16_to_cpu(de->rec_len) < OCFS2_DIR_REC_LEN(1))
> 			break;
> 		i += le16_to_cpu(de->rec_len);
> 	}
> 
> ocfs2_validate_inode_block() keeps i_size inside the inline area:
> 
> 	if (le16_to_cpu(data->id_count) >
> 	    ocfs2_max_inline_data_with_xattr(sb, di))
> 	if (le64_to_cpu(di->i_size) > le16_to_cpu(data->id_count))
> 
> and that area runs to the end of the inode block, so for a full inline
> directory data->id_data + i_size is the end of di_bh->b_data.  A bogus
> rec_len leaves i in the last OCFS2_DIR_REC_LEN(1) - 1 bytes of it, and
> de->rec_len, at byte offset 8 within the entry, is then read past the
> block.
> 
> The emit loop below hands i_size_read(inode) to ocfs2_check_dir_entry(),
> which refuses both an entry that close to the end and one whose rec_len
> runs past it.  Apply the same two bounds to the re-validation scan, reading
> i_size once into a local as ocfs2_check_dir_entry() takes it as @size.
> 
> Unlike the extent case there is no mask to corrupt here: an unbounded i
> only sets ctx->pos past i_size, which ends the readdir early rather
> than moving it to the wrong place.
> 
> Cc: Mark Fasheh <mark@fasheh.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Cc: Changwei Ge <gechangwei@live.cn>
> Cc: Jun Piao <piaojun@huawei.com>
> Cc: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/dir.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 39ce60874e17..863c69dec7da 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1812,7 +1812,12 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  		 * dirent right now.  Scan from the start of the block
>  		 * to make sure. */
>  		if (!inode_eq_iversion(inode, *f_version)) {
> -			for (i = 0; i < i_size_read(inode) && i < offset; ) {
> +			loff_t size = i_size_read(inode);
> +
> +			for (i = 0; i + OCFS2_DIR_REC_LEN(1) <= size &&
> +			     i < offset;) {
> +				unsigned int rec_len;
> +
>  				de = (struct ocfs2_dir_entry *)
>  					(data->id_data + i);
>  				/* It's too expensive to do a full
> @@ -1821,10 +1826,11 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>  				 * least that it is non-zero.  A
>  				 * failure will be detected in the
>  				 * dirent test below. */
> -				if (le16_to_cpu(de->rec_len) <
> -				    OCFS2_DIR_REC_LEN(1))
> +				rec_len = le16_to_cpu(de->rec_len);
> +				if (rec_len < OCFS2_DIR_REC_LEN(1) ||
> +				    i + rec_len > size)
>  					break;
> -				i += le16_to_cpu(de->rec_len);
> +				i += rec_len;
>  			}
>  			ctx->pos = offset = i;
>  			*f_version = inode_query_iversion(inode);