On Thu Feb 20 22:50:10 2025 +0530, Vikash Garodia wrote: > qsize represents size of shared queued between driver and video > firmware. Firmware can modify this value to an invalid large value. In > such situation, empty_space will be bigger than the space actually > available. Since new_wr_idx is not checked, so the following code will > result in an OOB write. > ... > qsize = qhdr->q_size > > if (wr_idx >= rd_idx) > empty_space = qsize - (wr_idx - rd_idx) > .... > if (new_wr_idx < qsize) { > memcpy(wr_ptr, packet, dwords << 2) --> OOB write > > Add check to ensure qsize is within the allocated size while > reading and writing packets into the queue. > > Cc: sta...@vger.kernel.org > Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files") > Reviewed-by: Bryan O'Donoghue <bryan.odonog...@linaro.org> > Signed-off-by: Vikash Garodia <quic_vgaro...@quicinc.com> > Signed-off-by: Hans Verkuil <hverk...@xs4all.nl>
Patch committed. Thanks, Hans Verkuil drivers/media/platform/qcom/venus/hfi_venus.c | 6 ++++++ 1 file changed, 6 insertions(+) --- diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c index a9167867063c..36d3a37db4e8 100644 --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -187,6 +187,9 @@ static int venus_write_queue(struct venus_hfi_device *hdev, /* ensure rd/wr indices's are read from memory */ rmb(); + if (qsize > IFACEQ_QUEUE_SIZE / 4) + return -EINVAL; + if (wr_idx >= rd_idx) empty_space = qsize - (wr_idx - rd_idx); else @@ -255,6 +258,9 @@ static int venus_read_queue(struct venus_hfi_device *hdev, wr_idx = qhdr->write_idx; qsize = qhdr->q_size; + if (qsize > IFACEQ_QUEUE_SIZE / 4) + return -EINVAL; + /* make sure data is valid before using it */ rmb();