Re: [PATCH v7 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] Missing reference counting on `iris_inst` objects allows an interrupt handler to use a freed instance.
--

--- Patch [13]: [PATCH v7 13/18] media: iris: update buffer requirements based on received info ---
commit fb9888a16075cc5bb0904df74da5c901575cc14f
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.

> 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);
>  		} else {
>  			struct hfi_msg_session_hdr_pkt *shdr;

[Severity: High]
This is a pre-existing issue, but since we are modifying the message
handling flow here, there appears to be a use-after-free race condition in
iris_hfi_gen1_handle_response().

Can an interrupt handler access a freed instance if iris_close() is called
concurrently?

If a user closes the device, iris_close() executes. Concurrently, the
threaded IRQ handler reads a message and looks up the instance:

IRQ Thread:
iris_hfi_gen1_handle_response() {
    ...
    inst = iris_get_instance(core, pkt->shdr.session_id);
    // iris_get_instance returns the pointer without acquiring a reference
    mutex_lock(&inst->lock); // blocks if iris_close() is holding the lock
    ...
}

User Thread:
iris_close() {
    ...
    mutex_lock(&inst->lock);
    iris_remove_session(inst);
    mutex_unlock(&inst->lock);
    mutex_destroy(&inst->lock);
    kfree(inst);
    ...
}

Once iris_close() frees the instance, the IRQ thread unblocks and attempts
to use the destroyed mutex and freed instance memory. Does the driver need
reference counting (e.g., kref) on the iris_inst structure to prevent it
from being freed while the IRQ thread is processing messages for it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-iris-ar50lt-v7-0-76af9dd4d1f6@oss.qualcomm.com?part=13