Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder
@ 2026-06-01 14:57 Junrui Luo
  2026-06-01 15:15 ` sashiko-bot
  2026-07-10  8:32 ` Hans Verkuil
  0 siblings, 2 replies; 5+ messages in thread
From: Junrui Luo @ 2026-06-01 14:57 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Yuhao Jiang,
	stable, Junrui Luo

vidioc_s_fmt_vid_out() sizes the encoder CAPTURE buffer from the
compressed descriptor pixfmt_fwht, whose sizeimage_mult is 3:
coded_w * coded_h * 3 + sizeof(struct fwht_cframe_hdr). fwht_encode_frame()
encodes one plane per component, and an incompressible plane takes the
FWHT_FRAME_UNENCODED path in encode_plane(), copying the plane verbatim.

For a 4-component pixel format all four planes are full resolution
(width_div == height_div == 1), so a frame that forces every plane
through the unencoded fallback writes
sizeof(struct fwht_cframe_hdr) + 4 * coded_w * coded_h bytes, overrunning
the plane by coded_w * coded_h, which can result in corruption
of adjacent kernel heap memory.

Bump pixfmt_fwht.sizeimage_mult from 3 to 4, matching the largest
components_num among the supported raw formats, so the capture buffer is
always large enough for the unencoded fallback.

Fixes: 29a7a5e99080 ("media: vicodec: add support for more pixel formats")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/media/test-drivers/vicodec/vicodec-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
index 318e8330f16a..3ff2d06742c2 100644
--- a/drivers/media/test-drivers/vicodec/vicodec-core.c
+++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
@@ -63,7 +63,7 @@ struct pixfmt_info {
 };
 
 static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
-	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
+	V4L2_PIX_FMT_FWHT, 0, 4, 1, 1, 1, 1, 1, 0, 1
 };
 
 static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {

---
base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
change-id: 20260601-fixes-7416dda13cfd

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder
  2026-06-01 14:57 [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder Junrui Luo
@ 2026-06-01 15:15 ` sashiko-bot
  2026-07-10  8:32 ` Hans Verkuil
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-01 15:15 UTC (permalink / raw)
  To: Junrui Luo; +Cc: linux-media

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix leaving the stateless decoder broken for 4-component unencoded frames due to insufficient sizeimage multiplier.
- [High] Data corruption in `decode_plane()` when processing uncompressed planes for interleaved pixel formats.
--

commit 9d6dcf75887aba570509c5de466de5d05f1da49d
Author: Junrui Luo <moonafterrain@outlook.com>

media: vicodec: fix out-of-bounds write in FWHT encoder

vidioc_s_fmt_vid_out() sizes the encoder CAPTURE buffer from the compressed
descriptor pixfmt_fwht, whose sizeimage_mult is 3. This patch bumps
pixfmt_fwht.sizeimage_mult from 3 to 4, matching the largest components_num
among the supported raw formats, so the capture buffer is always large
enough for the unencoded fallback.

> diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
> index 318e8330f16ae..3ff2d06742c20 100644
> --- a/drivers/media/test-drivers/vicodec/vicodec-core.c
> +++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
> @@ -63,7 +63,7 @@ struct pixfmt_info {
>  };
>  
>  static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
> -	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
> +	V4L2_PIX_FMT_FWHT, 0, 4, 1, 1, 1, 1, 1, 0, 1
>  };
>  
>  static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {

[Severity: High]
This isn't a bug introduced by this patch, but does leaving
pixfmt_stateless_fwht.sizeimage_mult at 3 break the stateless decoder for
4-component unencoded frames?

Since pixfmt_stateless_fwht.sizeimage_mult remains 3, vidioc_try_fmt() will
cap the sizeimage for compressed OUTPUT queue buffers at 3 * width * height.
If user space attempts to queue a valid 4-component unencoded frame, which
requires 4 * width * height bytes, will videobuf2 reject the buffer because
the payload exceeds the negotiated sizeimage?

[Severity: High]
This is a pre-existing issue, but looking at decode_plane() in
drivers/media/test-drivers/vicodec/codec-fwht.c, is there a data corruption
issue when processing uncompressed planes for interleaved pixel formats?

When falling back to decoding an uncompressed plane, the code uses:

	if (uncompressed) {
		int i;

		if (end_of_rlco_buf + 1 < *rlco + width * height / 2)
			return false;
		for (i = 0; i < height; i++) {
			memcpy(dst, *rlco, width);
			dst += dst_stride;
			*rlco += width / 2;
		}
		return true;
	}

Since this unpacking ignores the dst_step stride, which spaces out bytes for
interleaved formats like V4L2_PIX_FMT_RGB32 (dst_step = 4), does this
clobber the bytes belonging to other color components for the first
width / dst_step pixels of the row, and leave the rest of the row unwritten?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SYBPR01MB7881C44480F77338B6A93C61AF152@SYBPR01MB7881.ausprd01.prod.outlook.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder
  2026-06-01 14:57 [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder Junrui Luo
  2026-06-01 15:15 ` sashiko-bot
@ 2026-07-10  8:32 ` Hans Verkuil
  2026-07-11  6:29   ` Junrui Luo
  1 sibling, 1 reply; 5+ messages in thread
From: Hans Verkuil @ 2026-07-10  8:32 UTC (permalink / raw)
  To: Junrui Luo, Hans Verkuil, Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Yuhao Jiang,
	stable

Hi Junrui,

On 01/06/2026 16:57, Junrui Luo wrote:
> vidioc_s_fmt_vid_out() sizes the encoder CAPTURE buffer from the
> compressed descriptor pixfmt_fwht, whose sizeimage_mult is 3:
> coded_w * coded_h * 3 + sizeof(struct fwht_cframe_hdr). fwht_encode_frame()
> encodes one plane per component, and an incompressible plane takes the
> FWHT_FRAME_UNENCODED path in encode_plane(), copying the plane verbatim.
> 
> For a 4-component pixel format all four planes are full resolution
> (width_div == height_div == 1), so a frame that forces every plane
> through the unencoded fallback writes
> sizeof(struct fwht_cframe_hdr) + 4 * coded_w * coded_h bytes, overrunning
> the plane by coded_w * coded_h, which can result in corruption
> of adjacent kernel heap memory.
> 
> Bump pixfmt_fwht.sizeimage_mult from 3 to 4, matching the largest
> components_num among the supported raw formats, so the capture buffer is
> always large enough for the unencoded fallback.
> 
> Fixes: 29a7a5e99080 ("media: vicodec: add support for more pixel formats")

Actually, it's commit b40dc2bf3581 ("media: vicodec: add support for 4 new
RGB32 pixelformats") that introduced this.

Please update the Fixes tag.

> Reported-by: Yuhao Jiang <danisjiang@gmail.com>

Where was that reported? Is there a URL to the that report?

> Cc: stable@vger.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>  drivers/media/test-drivers/vicodec/vicodec-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
> index 318e8330f16a..3ff2d06742c2 100644
> --- a/drivers/media/test-drivers/vicodec/vicodec-core.c
> +++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
> @@ -63,7 +63,7 @@ struct pixfmt_info {
>  };
>  
>  static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
> -	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
> +	V4L2_PIX_FMT_FWHT, 0, 4, 1, 1, 1, 1, 1, 0, 1
>  };
>  
>  static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {

It should be changed here as well, otherwise the test-media regression script
in v4l-utils will fail.

Regards,

	Hans

> 
> ---
> base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
> change-id: 20260601-fixes-7416dda13cfd
> 
> Best regards,


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder
  2026-07-10  8:32 ` Hans Verkuil
@ 2026-07-11  6:29   ` Junrui Luo
  2026-07-11  8:54     ` Hans Verkuil
  0 siblings, 1 reply; 5+ messages in thread
From: Junrui Luo @ 2026-07-11  6:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Mauro Carvalho Chehab,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Yuhao Jiang, stable@vger.kernel.org

Hi Hans,

Thanks for the review.

On Fri, Jul 10, 2026 at 10:32:09AM +0200, Hans Verkuil wrote:
> Actually, it's commit b40dc2bf3581 ("media: vicodec: add support for 4 new
> RGB32 pixelformats") that introduced this.
> 
> Please update the Fixes tag.

I traced this back a bit further. Commit 16ecf6dff97c ("media:
vicodec: Add support for 4 planes formats") already added ARGB32 and
ABGR32 as four-component formats. Would this be more appropriate?

> 
> > Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> 
> Where was that reported? Is there a URL to the that report?

It was reported to me privately by Yuhao, so there is no public URL.

> 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> > ---
> >  drivers/media/test-drivers/vicodec/vicodec-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
> > index 318e8330f16a..3ff2d06742c2 100644
> > --- a/drivers/media/test-drivers/vicodec/vicodec-core.c
> > +++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
> > @@ -63,7 +63,7 @@ struct pixfmt_info {
> >  };
> >  
> >  static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
> > -	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
> > +	V4L2_PIX_FMT_FWHT, 0, 4, 1, 1, 1, 1, 1, 0, 1
> >  };
> >  
> >  static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {
> 
> It should be changed here as well, otherwise the test-media regression script
> in v4l-utils will fail.

I will update in v2.

Thanks,
Junrui Luo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder
  2026-07-11  6:29   ` Junrui Luo
@ 2026-07-11  8:54     ` Hans Verkuil
  0 siblings, 0 replies; 5+ messages in thread
From: Hans Verkuil @ 2026-07-11  8:54 UTC (permalink / raw)
  To: Junrui Luo
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Mauro Carvalho Chehab,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Yuhao Jiang, stable@vger.kernel.org

On 11/07/2026 08:29, Junrui Luo wrote:
> Hi Hans,
> 
> Thanks for the review.
> 
> On Fri, Jul 10, 2026 at 10:32:09AM +0200, Hans Verkuil wrote:
>> Actually, it's commit b40dc2bf3581 ("media: vicodec: add support for 4 new
>> RGB32 pixelformats") that introduced this.
>>
>> Please update the Fixes tag.
> 
> I traced this back a bit further. Commit 16ecf6dff97c ("media:
> vicodec: Add support for 4 planes formats") already added ARGB32 and
> ABGR32 as four-component formats. Would this be more appropriate?

Yes, you are right. That's when support for this was added.
Good catch.

> 
>>
>>> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
>>
>> Where was that reported? Is there a URL to the that report?
> 
> It was reported to me privately by Yuhao, so there is no public URL.
> 
>>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
>>> ---
>>>  drivers/media/test-drivers/vicodec/vicodec-core.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
>>> index 318e8330f16a..3ff2d06742c2 100644
>>> --- a/drivers/media/test-drivers/vicodec/vicodec-core.c
>>> +++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
>>> @@ -63,7 +63,7 @@ struct pixfmt_info {
>>>  };
>>>  
>>>  static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
>>> -	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
>>> +	V4L2_PIX_FMT_FWHT, 0, 4, 1, 1, 1, 1, 1, 0, 1
>>>  };
>>>  
>>>  static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {
>>
>> It should be changed here as well, otherwise the test-media regression script
>> in v4l-utils will fail.
> 
> I will update in v2.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-11  8:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01 14:57 [PATCH] media: vicodec: fix out-of-bounds write in FWHT encoder Junrui Luo
2026-06-01 15:15 ` sashiko-bot
2026-07-10  8:32 ` Hans Verkuil
2026-07-11  6:29   ` Junrui Luo
2026-07-11  8:54     ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox