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

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: [email protected]
> Signed-off-by: Joseph Qi <[email protected]>

Reviewed-and-tested-by: Dmitry Antipov <[email protected]>

> ---
>   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";