Re: [PATCH] media: vicodec: fix out-of-bounds write on capture buffer

Le lundi 03 août 2026 à 12:44 +0000, Junrui Luo a écrit :
> On Wed, Jul 15, 2026 at 08:21:50PM -0400, Nicolas Dufresne wrote:
> > The analyses is just incorrect. At this point, the driver is entering DRC,
> > meaning the queue will be drain and will stop until streamoff/on cycle happen.  
> > If there is a out-of-bound bug, it would be that that q_data->sizeimage is not
> > match to be smaller or equal to the vb2 buffer size at streamon time, not during
> > processing.
> > 
> > NAK
> > 
> > Nicolas
> 
> Thanks for looking. Right, `source_changed` gates the draining during the
> streamoff/on cycle.
> 
> However, `source_changed` is only set in job_ready(). The first source change
> goes through vicodec_buf_queue(), which sets `first_source_change_sent` but not
> `source_changed`, so it does not drain. A non-conforming application could
> commit CAPTURE resources before calling STREAMON(OUTPUT).
> 
> On v7.0-rc3, KASAN:
> [    3.777164] BUG: KASAN: slab-out-of-bounds in device_run+0x969/0x1850
> [    3.779180] Write of size 2560 at addr ffff888008de1000 by task trigger/69
> 
> It can be triggered by S_FMT(CAPTURE, GREY 1280x720), CREATE_BUFS(CAPTURE) [plane
> over-allocated], STREAMON(CAPTURE). Then QBUF(OUTPUT) with an FWHT
> header for the same 1280x720 but NV24 (3 components, full chroma). That
> grows q_dst->sizeimage to 2764800 without changing coded_width/height, so
> STREAMON(OUTPUT) runs device_run() and copy_cap_to_ref() memcpy()s the
> NV24 chroma plane past the GREY-sized ref_frame.buf.
> 
> As for the fix, where would you prefer the guard? I'll send a v2 accordingly.

The sizeimage, if used by device_run() should only be updated once the drain has
completed. In DRC, there should be no fault caused by the above. Once driver
process the NV24 header, it should emit the SRC_CH event and start the draining
process (while draining, the capture format should not change). Application
should keep DQ(capture) until it sees the LAST flag. And then its the right time
to G_FMT(capture) (optionally enumerate) and reallocate the capture queue. That
imply cycling the capture through streamoff/on, which should validate the size
image against the frame header format.

Nicolas

- 

> 
> Thanks,
> Junrui Luo

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCantCmgAKCRDZQZRRKWBy
9FEgAP4l9Dx88FHLTWdHbwBWWmUof6T4JAlAIy/D340xdOa3BwEAkKGKXvJ4kK9X
MK14VPtEbjLsGDFLQTSE6+0rsoPIBAY=
=IGjc
-----END PGP SIGNATURE-----