Re: [PATCH v5 11/16] 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] Concurrent handling of HFI response packets and instance teardown can result in a Use-After-Free (UAF) of `inst`.
--

commit 39194b5e5ad26925b8c03d02889401ec6e1fa21d
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.
This will be used for upcoming support of AR50Lt platforms.

> 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..896953ea62ea4 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 +730,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);
>  		} else {
>  			struct hfi_msg_session_hdr_pkt *shdr;

[Severity: High]
This is a pre-existing issue, but can concurrent handling of HFI response
packets and instance teardown result in a use-after-free of the inst pointer?

Looking at the start of iris_hfi_gen1_handle_response(), it calls
iris_get_instance() which retrieves the inst pointer from the core list
without incrementing any reference count:

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

The asynchronous response handler then blocks trying to acquire
mutex_lock(&inst->lock).

Concurrently, if iris_close() executes and calls iris_session_close(), it
can temporarily unlock inst->lock while waiting for the final 
HFI_MSG_SYS_SESSION_END response. This allows iris_close() to eventually
finish, remove the session, unlock and destroy the mutex, and free the
memory:

drivers/media/platform/qcom/iris/iris_vidc.c:iris_close() {
    iris_remove_session(inst);
    mutex_unlock(&inst->lock);
    mutex_destroy(&inst->ctx_q_lock);
    mutex_destroy(&inst->lock);
    ...
    kfree(inst);
}

If a spurious or trailing firmware message is processed for the same session,
the response handler could wake up when the lock is released by iris_close(),
successfully acquire the destroyed lock, and dereference the freed inst memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=11