Hi Bryan,
On 2/16/2025 9:33 PM, Bryan O'Donoghue wrote:
On 15/02/2025 17:19, Vedang Nagar wrote:
This series primarily adds check at relevant places in venus driver
where there are possible OOB accesses due to unexpected payload
from venus firmware. The patches describes the specific OOB possibility.
Signed-off-by: Vedang Nagar <quic_vnagar@xxxxxxxxxxx>
---
Changes in v2:
- Decompose sequence change event function.
- Fix repopulating the packet .with the first read during read_queue.
- Link to v1: https://lore.kernel.org/r/20250104-venus-security-fixes-
v1-0-9d0dd4594cb4@xxxxxxxxxxx
---
Vedang Nagar (2):
media: venus: fix OOB read issue due to double read
media: venus: fix OOB access issue while reading sequence
changed events
drivers/media/platform/qcom/venus/hfi_msgs.c | 72 +++++++++++++++++
++++++----
drivers/media/platform/qcom/venus/hfi_venus.c | 1 +
2 files changed, 63 insertions(+), 10 deletions(-)
---
base-commit: 91e71d606356e50f238d7a87aacdee4abc427f07
change-id: 20241211-venus-security-fixes-50c22e2564d5
Best regards,
Could you please address the feedback I gave you / questions posited in
these two messages ?
4cfc1fe1-2fab-4256-9ce2-b4a0aad1069e@xxxxxxxxxx
0eab7323-ce86-40c7-9737-06eedcdf492d@xxxxxxxxxx
The basic question : what is the lifetime of the data from RX interrupt
to consumption by another system agent, DSP, userspace, whatever ?
As mentioned in [1], With the regular firmware, after RX interrupt the
data can be considered as valid until next interrupt is raised, but with
the rouge firmware, data can get invalid during the second read and our
intention is to avoid out of bound access read because of such issues.
[1]:
https://lore.kernel.org/lkml/4cfc1fe1-2fab-4256-9ce2-b4a0aad1069e@xxxxxxxxxx/T/#m5f1737b16e68f8b8fc1d75517356b6566d0ec619
Why is it in this small specific window that the data can change but not
later ? What is the mechanism the data can change and how do the changes
you propose here address the data lifetime problem ?
Currently this issue has been discovered by external researchers at this
point, but if any such OOB issue is discovered at later point as well
then we shall fix them as well.
Also, with rougue firmware we cannot fix the data lifetime problem in my
opinion, but atleast we can fix the out of bound issues.
Without that context, I don't believe it is really possible to validate
an additional memcpy() here and there in the code as fixing anything.
There is no additional memcpy() now in the v2 patch, but as part of the
fix, we are just trying to retain the length of the packet which was
being read in the first memcpy() to avoid the OOB read access.
Please let me know if you have any other suggestions.
Regards,
Vedang Nagar
---
bod
