Re: [PATCH v4 10/16] media: iris: add minimal GET_PROPERTY implementation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Possible Use-After-Free in firmware response handler due to lockless instance lookup and lack of reference counting.
--

commit ea864f98c9ad1ba57f2027e2f0fc7f82f044f629
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

media: iris: add minimal GET_PROPERTY implementation

AR50Lt with the Gen1 firmware requires host to read
HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS property, otherwie it doesn't
update internal data and fails the HFI_CMD_SESSION_LOAD_RESOURCES
command. Implement minimal support for querying the properties from the
firmware.

> AR50Lt with the Gen1 firmware requires host to read
> HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS property, otherwie it doesn't

This isn't a bug, but there's a typo (otherwie) in the commit message.

> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index bfd7495bf44f0..23fc7194b1e3a 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
[ ... ]
> @@ -652,6 +656,8 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
>  			iris_hfi_gen1_session_etb_done(inst, hdr);
>  		} else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
>  			iris_hfi_gen1_session_ftb_done(inst, hdr);
> +		} else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> +			complete(&inst->completion);

[Severity: High]
This is a pre-existing issue, but could there be a use-after-free race condition
when handling firmware responses here?

Earlier in iris_hfi_gen1_handle_response(), the instance is looked up using
iris_get_instance():

	inst = iris_get_instance(core, pkt->shdr.session_id);
	...
	mutex_lock(&inst->lock);

iris_get_instance() finds the instance in the core->instances list and returns
a raw pointer after dropping core->lock, without incrementing a reference count.

Concurrently, if a userspace process closes the device node, iris_close() might
remove the instance from the list, unlock inst->lock, destroy the mutex, and
call kfree(inst).

If the response handler thread calls iris_get_instance() just before iris_close()
removes the instance from the list, the response thread could receive the pointer
and block on mutex_lock(&inst->lock) while iris_close() holds it.

After iris_close() finishes teardown and frees the instance, the response thread
would be unblocked and attempt to access the freed memory and destroyed mutex.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-iris-ar50lt-v4-0-0abfb74d5b3c@oss.qualcomm.com?part=10