Re: [PATCH] ocfs2: update xattr count before moving bucket entries

[Heming Zhao <heming.zhao@suse.com>]

On Tue, Sep 15, 2026 at 12:16:33PM +0800, Su Yue wrote:
> The xh_entries array is annotated with __counted_by_le(xh_count), so
> FORTIFY uses xh_count to determine its bounds. When inserting an entry
> into a bucket, ocfs2_xa_bucket_add_entry() shifts existing entries before
> incrementing xh_count. The destination therefore extends one entry past
> the bounds described by the old count.
> 
> With CONFIG_CC_HAS_COUNTED_BY and CONFIG_FORTIFY_SOURCE enabled,
> ocfs2-test: single_run-WIP.sh -t reflink triggers the following failure while
> adding an extended attribute:
> 
> [  150.156484] memmove: detected buffer overflow: 352 byte write of buffer size 336
> [  150.156487] WARNING: lib/string_helpers.c:1036 at __fortify_report+0x3d/0x50, CPU#15: reflink_test/2336
> [  150.160496] RIP: 0010:__fortify_report+0x40/0x50
> [  150.164031] Call Trace:
> [  150.164141]  <TASK>
> [  150.164232]  __fortify_panic+0x9/0xb
> [  150.164383]  ocfs2_xa_bucket_add_entry.cold+0x17/0x28 [ocfs2]
> [  150.164661]  ocfs2_xa_set+0x8fb/0xf40 [ocfs2]
> 
> Increment xh_count before memmove() so the destination bounds include
> the new entry. Keep the local count unchanged to calculate the insertion
> position and move length from the original number of entries. The caller
> has already checked that there is enough space for the new entry.
> 
> Fixes: 2f26f58df041b ("ocfs2: annotate flexible array members with __counted_by_le()")

The code looks good to me. Just one note:
commit 2f26f58df041b introduced stricter bounds checking rather than fixing a bug.
This reference should go into the commit log instead of a Fixes tag.

Thanks,
Heming
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
>  fs/ocfs2/xattr.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 35bcbb0ff607b..49d82ea5a1271 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -2076,12 +2076,17 @@ static void ocfs2_xa_bucket_add_entry(struct ocfs2_xa_loc *loc, u32 name_hash)
>  		}
>  	}
>  
> +	/*
> +	 * Increment xh_count before memmove() so __counted_by_le(xh_count)
> +	 * includes the new entry in the destination bounds.
> +	 */
> +	le16_add_cpu(&xh->xh_count, 1);
> +
>  	if (low != count)
>  		memmove(&xh->xh_entries[low + 1],
>  			&xh->xh_entries[low],
>  			((count - low) * sizeof(struct ocfs2_xattr_entry)));
>  
> -	le16_add_cpu(&xh->xh_count, 1);
>  	loc->xl_entry = &xh->xh_entries[low];
>  	memset(loc->xl_entry, 0, sizeof(struct ocfs2_xattr_entry));
>  }
> -- 
> 2.55.0
>