On 05/11/2024 08:54, 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: stable@vger.kernel.org Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files") Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com
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 f9437b6412b91c2483670a2b11f4fd43f3206404..50d92214190d88eff273a5ba3f95486f758bcc05 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();
You have this same calculation in venus_set_qhdr_defaults() really needs a macro or something to stop repeating the same code in another patch later.
Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org