Re: [PATCH v8 13/18] media: iris: update buffer requirements based on received info

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free in threaded IRQ handler due to missing reference counting on iris_inst
--

--- Patch [13]: [PATCH v8 13/18] media: iris: update buffer requirements based on received info ---
commit eebc551c83e572d39b479f0c60f9705bbd9fe71d
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

media: iris: update buffer requirements based on received info
    
Upon receiving data for HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS the
driver should update buffer sizes and counts from the received data.
Implement corresponding functionality updating buffers data. This will
be used for upcoming support of AR50Lt platforms with Gen1 firmware.

> 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 23fc7194b1e3a..7ad6f0bb46770 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> [ ... ]
> @@ -657,7 +731,7 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
>  		} 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);
> +			iris_hfi_gen1_session_property_info(inst, hdr);

[Severity: High]
This is a pre-existing issue, but could this inst dereference lead to a
use-after-free if the IRQ thread races with userspace calling close(fd)?

Looking at iris_get_instance() in
drivers/media/platform/qcom/iris/iris_utils.c, it drops core->lock and
returns a raw instance pointer without incrementing a reference count:

drivers/media/platform/qcom/iris/iris_utils.c:iris_get_instance() {
    ...
	mutex_lock(&core->lock);
	list_for_each_entry(inst, &core->instances, list) {
		if (inst->session_id == session_id) {
			mutex_unlock(&core->lock);
			return inst;
		}
	}

	mutex_unlock(&core->lock);
	return NULL;
}

Concurrently, if a userspace thread calls close(fd), iris_close() in
drivers/media/platform/qcom/iris/iris_vidc.c calls iris_remove_session(inst)
and then kfree(inst), without synchronizing with concurrent readers in the IRQ
thread.

If the threaded IRQ handler gets preempted after iris_get_instance()
returns but before acquiring inst->lock, would it wake up and
dereference the freed inst pointer when calling mutex_lock(&inst->lock)
earlier in this function?

>  		} else {
>  			struct hfi_msg_session_hdr_pkt *shdr;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-iris-ar50lt-v8-0-6597c08cea6f@oss.qualcomm.com?part=13