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

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

[Haotian Zhang]

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")
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]);
 
 	print_ref_list_b(builder, b0_reflist, 0);
-- 
2.43.0

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

[Nicolas Dufresne]

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

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.

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 --]

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

[Nicolas Dufresne]

[-- 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 --]

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

[Nicolas Dufresne]

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

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

One last comment, I think this would be more consistent with the memcpy() calls:

+			 sizeof(b1_reflist[0]) * builder->num_valid))

Nicolas

>  		swap(b1_reflist[0], b1_reflist[1]);
>  
>  	print_ref_list_b(builder, b0_reflist, 0);

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