Re: [PATCH] media: v4l2-h264: Fix memcmp() size in B1 reference list comparison

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]

Le vendredi 28 août 2026 à 14:31 -0400, Nicolas Dufresne a écrit :
> Le vendredi 28 août 2026 à 10:17 +0800, Haotian Zhang a écrit :
> > In v4l2_h264_build_b_ref_lists(), the B0/B1 list equality
> > check passes the entry count builder->num_valid to memcmp()
> > instead of a byte size. Since struct v4l2_h264_reference is
> > two bytes (fields and index), only half of each list is
> > compared, so distinct lists can be wrongly treated as equal
> > and trigger an incorrect swap(b1_reflist[0], b1_reflist[1]).
> > 
> > Change the memcmp() size argument to builder->num_valid *
> > sizeof(*b1_reflist) so that the full byte length of both
> > reference lists is compared.
> > 
> > Fixes: 624922a2739b ("media: v4l2-core: Add helpers to build the H264 P/B0/B1 reflists")
> 
> This is the wrong commit. It was fine until I broke it 4 years ago it seems.
> Boris don't do that kind of silly mistake :-S
> 
> commit 2e2c3d6c0ef88ffac0d6b5079ee88cf8408f5f3b
> Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Date:   Fri May 13 22:29:03 2022 +0200
> 
>     media: h264: Use v4l2_h264_reference for reflist
> 
> 
> > Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> > ---
> >  drivers/media/v4l2-core/v4l2-h264.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
> > index c00197d095e7..1b1421caf488 100644
> > --- a/drivers/media/v4l2-core/v4l2-h264.c
> > +++ b/drivers/media/v4l2-core/v4l2-h264.c
> > @@ -440,7 +440,8 @@ v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
> >  	}
> >  
> >  	if (builder->num_valid > 1 &&
> > -	    !memcmp(b1_reflist, b0_reflist, builder->num_valid))
> > +	    !memcmp(b1_reflist, b0_reflist,
> > +			builder->num_valid * sizeof(*b1_reflist)))
> >  		swap(b1_reflist[0], b1_reflist[1]);
> 
> And of course this swap need to be replaced.

Ignore this one, I should have read the implementation of the swap macro ... its
working fine for this case:

   do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)

> 
> Nicolas
> 
> >  
> >  	print_ref_list_b(builder, b0_reflist, 0);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]