linux.git/drivers/media/platform/qcom, branch v6.7

media: venus: Fix firmware path for resources

2023-10-23T10:24:53+00:00

The firmware path for some of the resources is still the old format. This
fixes the path to address the firmware correctly using the new .mbn
format.

Signed-off-by: Patrick Whewell 
Signed-off-by: Stanimir Varbanov 
Signed-off-by: Hans Verkuil

media: venus: hfi_cmds: Replace one-element array with flex-array member and use __counted_by

2023-10-23T10:24:52+00:00

Array `data` in `struct hfi_sfr` is being used as a fake flexible array
at run-time:

drivers/media/platform/qcom/venus/hfi_venus.c:
1033         p = memchr(sfr->data, '\0', sfr->buf_size);
1034         /*
1035          * SFR isn't guaranteed to be NULL terminated since SYS_ERROR indicates
1036          * that Venus is in the process of crashing.
1037          */
1038         if (!p)
1039                 sfr->data[sfr->buf_size - 1] = '\0';
1040
1041         dev_err_ratelimited(dev, "SFR message from FW: %s\n", sfr->data);

Fake flexible arrays are deprecated, and should be replaced by
flexible-array members. So, replace one-element array with a
flexible-array member in `struct hfi_sfr`.

While there, also annotate array `data` with __counted_by() to prepare
for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).

This results in no differences in binary output.

This issue was found with the help of Coccinelle, and audited and fixed
manually.

Signed-off-by: Gustavo A. R. Silva 
Reviewed-by: Kees Cook 
Signed-off-by: Stanimir Varbanov 
Signed-off-by: Hans Verkuil

media: venus: hfi_parser: Add check to keep the number of codecs within range

2023-10-23T10:24:52+00:00

Supported codec bitmask is populated from the payload from venus firmware.
There is a possible case when all the bits in the codec bitmask is set. In
such case, core cap for decoder is filled  and MAX_CODEC_NUM is utilized.
Now while filling the caps for encoder, it can lead to access the caps
array beyong 32 index. Hence leading to OOB write.
The fix counts the supported encoder and decoder. If the count is more than
max, then it skips accessing the caps.

Cc: stable@vger.kernel.org
Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser")
Signed-off-by: Vikash Garodia 
Signed-off-by: Stanimir Varbanov 
Signed-off-by: Hans Verkuil

media: venus: hfi: add checks to handle capabilities from firmware

2023-10-23T10:24:52+00:00

The hfi parser, parses the capabilities received from venus firmware and
copies them to core capabilities. Consider below api, for example,
fill_caps - In this api, caps in core structure gets updated with the
number of capabilities received in firmware data payload. If the same api
is called multiple times, there is a possibility of copying beyond the max
allocated size in core caps.
Similar possibilities in fill_raw_fmts and fill_profile_level functions.

Cc: stable@vger.kernel.org
Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser")
Signed-off-by: Vikash Garodia 
Signed-off-by: Stanimir Varbanov 
Signed-off-by: Hans Verkuil

media: venus: hfi: fix the check to handle session buffer requirement

2023-10-23T10:24:52+00:00

Buffer requirement, for different buffer type, comes from video firmware.
While copying these requirements, there is an OOB possibility when the
payload from firmware is more than expected size. Fix the check to avoid
the OOB possibility.

Cc: stable@vger.kernel.org
Fixes: 09c2845e8fe4 ("[media] media: venus: hfi: add Host Firmware Interface (HFI)")
Reviewed-by: Nathan Hebert 
Signed-off-by: Vikash Garodia 
Signed-off-by: Stanimir Varbanov 
Signed-off-by: Hans Verkuil

media: venus: hfi: add checks to perform sanity on queue pointers

2023-10-23T10:24:52+00:00

Read and write pointers are used to track the packet index in the memory
shared between video driver and firmware. There is a possibility of OOB
access if the read or write pointer goes beyond the queue memory size.
Add checks for the read and write pointer to avoid OOB access.

Cc: stable@vger.kernel.org
Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
Signed-off-by: Vikash Garodia 
Signed-off-by: Stanimir Varbanov 
Signed-off-by: Hans Verkuil

media: qcom: venus: fix incorrect return value

2023-10-12T08:12:54+00:00

'pd' can be NULL, and in that case it shouldn't be passed to
PTR_ERR. Fixes a smatch warning:

drivers/media/platform/qcom/venus/pm_helpers.c:873 vcodec_domains_get() warn: passing zero to 'PTR_ERR'

Signed-off-by: Hans Verkuil 
Reviewed-by: Bryan O'Donoghue

media: qcom: camss: Comment CSID dt_id field

2023-10-07T08:55:44+00:00

Digging into the documentation we find that the DT_ID bitfield is used to
map the six bit DT to a two bit ID code. This value is concatenated to the
VC bitfield to create a CID value. DT_ID is the two least significant bits
of CID and VC the most significant bits.

Originally we set dt_id = vc * 4 in and then subsequently set dt_id = vc.

commit 3c4ed72a16bc ("media: camss: sm8250: Virtual channels for CSID")
silently fixed the multiplication by four which would give a better
value for the generated CID without mentioning what was being done or why.

Next up I haplessly changed the value back to "dt_id = vc * 4" since there
didn't appear to be any logic behind it.

Hans asked what the change was for and I honestly couldn't remember the
provenance of it, so I dug in.

Link: https://lore.kernel.org/linux-arm-msm/edd4bf9b-0e1b-883c-1a4d-50f4102c3924@xs4all.nl/

Add a comment so the next hapless programmer doesn't make this same
mistake.

Signed-off-by: Bryan O'Donoghue 
Signed-off-by: Hans Verkuil

media: qcom: camss: Propagate vfe_reset error up the callstack

2023-10-07T08:55:44+00:00

A previous patch I had removed the returns from vfe_disable() since we
didn't trap any meaningful errors. Konrad pointed out vfe_reset() could
return an error, which is true.

Trap the vfe_reset() error code and throw it up the callstack.

Suggested-by: Konrad Dybcio 
Signed-off-by: Bryan O'Donoghue 
Signed-off-by: Hans Verkuil

media: qcom: camss: Move vfe_disable into a common routine where applicable

2023-10-07T08:55:44+00:00

We can move vfe_disable() into a common routine in the core VFE file
provided we make wm_stop() a VFE specific callback.

The callback is required to capture the case where VFE 17x currently isn't
VC enabled where as VFE 480 is.

Suggested-by: Laurent Pinchart 
Signed-off-by: Bryan O'Donoghue 
Signed-off-by: Hans Verkuil