Re: [RESEND PATCH 1/2] ext4: introduce ITAIL helper

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Feb 07, 2025 at 11:27:42AM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Introduce ITAIL helper to get the bound of xattr in inode.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/xattr.c | 10 +++++-----
>  fs/ext4/xattr.h |  3 +++
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 7647e9f6e190..0e4494863d15 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -649,7 +649,7 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
>  		return error;
>  	raw_inode = ext4_raw_inode(&iloc);
>  	header = IHDR(inode, raw_inode);
> -	end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> +	end = ITAIL(inode, raw_inode);
>  	error = xattr_check_inode(inode, header, end);
>  	if (error)
>  		goto cleanup;
> @@ -793,7 +793,7 @@ ext4_xattr_ibody_list(struct dentry *dentry, char *buffer, size_t buffer_size)
>  		return error;
>  	raw_inode = ext4_raw_inode(&iloc);
>  	header = IHDR(inode, raw_inode);
> -	end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> +	end = ITAIL(inode, raw_inode);
>  	error = xattr_check_inode(inode, header, end);
>  	if (error)
>  		goto cleanup;
> @@ -879,7 +879,7 @@ int ext4_get_inode_usage(struct inode *inode, qsize_t *usage)
>  			goto out;
>  		raw_inode = ext4_raw_inode(&iloc);
>  		header = IHDR(inode, raw_inode);
> -		end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> +		end = ITAIL(inode, raw_inode);
>  		ret = xattr_check_inode(inode, header, end);
>  		if (ret)
>  			goto out;
> @@ -2235,7 +2235,7 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
>  	header = IHDR(inode, raw_inode);
>  	is->s.base = is->s.first = IFIRST(header);
>  	is->s.here = is->s.first;
> -	is->s.end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> +	is->s.end = ITAIL(inode, raw_inode);
>  	if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
>  		error = xattr_check_inode(inode, header, is->s.end);
>  		if (error)
> @@ -2786,7 +2786,7 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
>  	 */
>  
>  	base = IFIRST(header);
> -	end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> +	end = ITAIL(inode, raw_inode);
>  	min_offs = end - base;
>  	total_ino = sizeof(struct ext4_xattr_ibody_header) + sizeof(u32);
>  
> diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
> index b25c2d7b5f99..d331bd636480 100644
> --- a/fs/ext4/xattr.h
> +++ b/fs/ext4/xattr.h
> @@ -67,6 +67,9 @@ struct ext4_xattr_entry {
>  		((void *)raw_inode + \
>  		EXT4_GOOD_OLD_INODE_SIZE + \
>  		EXT4_I(inode)->i_extra_isize))
> +#define ITAIL(inode, raw_inode) \
> +	((void *)raw_inode + \
> +	 EXT4_SB(inode->i_sb)->s_inode_size)

These are macros, you ought to wrap the arguments in parentheses to
avoid subtle bugs:

#define ITAIL(inode, raw_inode) \
	((void *)(raw_inode) + \
	 EXT4_SB((inode)->i_sb)->s_inode_size)

(or maybe just a static inline helper, but I guess we're passing around
void pointers so meh)

--D

>  #define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
>  
>  /*
> -- 
> 2.34.1
> 
>