[PATCH] ocfs2: fix boundary check in ocfs2_check_dir_entry() to use buffer offset

[PATCH] ocfs2: fix boundary check in ocfs2_check_dir_entry() to use buffer offset

[Joseph Qi]

Commit 390ac56cf0f6 ("ocfs2: add boundary check to ocfs2_check_dir_entry()")
added an out-of-bounds guard using the caller-supplied 'offset' argument:

	if (offset > size - OCFS2_DIR_REC_LEN(1))
		return 0;

However, 'offset' and 'size' are not measured against the same base for
all callers. In the block-based lookup path, ocfs2_find_entry_el() passes
'offset' as an absolute offset into the whole directory:

	i = ocfs2_search_dirblock(bh, dir, name, namelen,
				  block << sb->s_blocksize_bits,
				  bh->b_data, sb->s_blocksize, res_dir);

while 'size' is a single block size (sb->s_blocksize). For any directory
entry located in the second or later block, 'offset' is >= sb->s_blocksize,
so the guard rejects every such entry even though it is perfectly valid and
lies entirely within its block buffer.

This makes mounting fail for filesystems whose system directory spans more
than one block, e.g. a volume formatted with a small block size:

  mkfs.ocfs2 -b 512 -C 4096 -N 2 -T datafiles --fs-features=usrquota,grpquota

  ocfs2_check_dir_entry:314 ERROR: directory entry (#18: offset=512) too close to end or out-of-bounds
  ocfs2_init_local_system_inodes:496 ERROR: status=-22, sysfile=12, slot=0
  ocfs2_mount_volume:1757 ERROR: status = -22

The dirent's position within the buffer being validated is
((char *)de - buf), which is what the rest of the function already uses
(via next_offset) and what must be bounds-checked against 'size'. Compute
that buffer-relative offset and use it for the guard. The subtraction is
reordered to size - buf_offset < OCFS2_DIR_REC_LEN(1) to avoid an unsigned
underflow when size is smaller than the minimal record length.

Fixes: 390ac56cf0f6 ("ocfs2: add boundary check to ocfs2_check_dir_entry()")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/dir.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index 8e6b032383278..d7fc3cccf2f4b 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -302,10 +302,11 @@ static int ocfs2_check_dir_entry(struct inode *dir,
 				 unsigned long offset)
 {
 	const char *error_msg = NULL;
+	unsigned long buf_offset = (char *)de - buf;
 	unsigned long next_offset;
 	int rlen;
 
-	if (offset > size - OCFS2_DIR_REC_LEN(1)) {
+	if (buf_offset > size || size - buf_offset < OCFS2_DIR_REC_LEN(1)) {
 		/* Dirent is (maybe partially) beyond the buffer
 		 * boundaries so touching 'de' members is unsafe.
 		 */
@@ -316,7 +317,7 @@ static int ocfs2_check_dir_entry(struct inode *dir,
 	}
 
 	rlen = le16_to_cpu(de->rec_len);
-	next_offset = ((char *) de - buf) + rlen;
+	next_offset = buf_offset + rlen;
 
 	if (unlikely(rlen < OCFS2_DIR_REC_LEN(1)))
 		error_msg = "rec_len is smaller than minimal";
-- 
2.39.3

Re: [PATCH] ocfs2: fix boundary check in ocfs2_check_dir_entry() to use buffer offset

[Dmitry Antipov]

On 7/10/26 7:05 AM, Joseph Qi wrote:

> Commit 390ac56cf0f6 ("ocfs2: add boundary check to ocfs2_check_dir_entry()")
> added an out-of-bounds guard using the caller-supplied 'offset' argument:
> 
> 	if (offset > size - OCFS2_DIR_REC_LEN(1))
> 		return 0;
> 
> However, 'offset' and 'size' are not measured against the same base for
> all callers. In the block-based lookup path, ocfs2_find_entry_el() passes
> 'offset' as an absolute offset into the whole directory:
> 
> 	i = ocfs2_search_dirblock(bh, dir, name, namelen,
> 				  block << sb->s_blocksize_bits,
> 				  bh->b_data, sb->s_blocksize, res_dir);
> 
> while 'size' is a single block size (sb->s_blocksize). For any directory
> entry located in the second or later block, 'offset' is >= sb->s_blocksize,
> so the guard rejects every such entry even though it is perfectly valid and
> lies entirely within its block buffer.
> 
> This makes mounting fail for filesystems whose system directory spans more
> than one block, e.g. a volume formatted with a small block size:
> 
>    mkfs.ocfs2 -b 512 -C 4096 -N 2 -T datafiles --fs-features=usrquota,grpquota
> 
>    ocfs2_check_dir_entry:314 ERROR: directory entry (#18: offset=512) too close to end or out-of-bounds
>    ocfs2_init_local_system_inodes:496 ERROR: status=-22, sysfile=12, slot=0
>    ocfs2_mount_volume:1757 ERROR: status = -22
> 
> The dirent's position within the buffer being validated is
> ((char *)de - buf), which is what the rest of the function already uses
> (via next_offset) and what must be bounds-checked against 'size'. Compute
> that buffer-relative offset and use it for the guard. The subtraction is
> reordered to size - buf_offset < OCFS2_DIR_REC_LEN(1) to avoid an unsigned
> underflow when size is smaller than the minimal record length.
> 
> Fixes: 390ac56cf0f6 ("ocfs2: add boundary check to ocfs2_check_dir_entry()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

Reviewed-and-tested-by: Dmitry Antipov <dmantipov@yandex.ru>

> ---
>   fs/ocfs2/dir.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 8e6b032383278..d7fc3cccf2f4b 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -302,10 +302,11 @@ static int ocfs2_check_dir_entry(struct inode *dir,
>   				 unsigned long offset)
>   {
>   	const char *error_msg = NULL;
> +	unsigned long buf_offset = (char *)de - buf;
>   	unsigned long next_offset;
>   	int rlen;
>   
> -	if (offset > size - OCFS2_DIR_REC_LEN(1)) {
> +	if (buf_offset > size || size - buf_offset < OCFS2_DIR_REC_LEN(1)) {
>   		/* Dirent is (maybe partially) beyond the buffer
>   		 * boundaries so touching 'de' members is unsafe.
>   		 */
> @@ -316,7 +317,7 @@ static int ocfs2_check_dir_entry(struct inode *dir,
>   	}
>   
>   	rlen = le16_to_cpu(de->rec_len);
> -	next_offset = ((char *) de - buf) + rlen;
> +	next_offset = buf_offset + rlen;
>   
>   	if (unlikely(rlen < OCFS2_DIR_REC_LEN(1)))
>   		error_msg = "rec_len is smaller than minimal";