Re: [PATCH 3/3] media: iris: Fix use-after-free on unbind with open sessions

[Vishnu Reddy <busanna.reddy@oss.qualcomm.com>]

On 11/9/2026 4:52 PM, Bryan O'Donoghue wrote:
> On 09/11/2026 05:08, Vishnu Reddy wrote:
>>>> +static void iris_core_release(struct v4l2_device *v4l2_dev)
>>>> +{
>>>> +    struct iris_core *core = container_of(v4l2_dev, struct iris_core,
>>>> v4l2_dev);
>>>> +
>>>> +    mutex_destroy(&core->lock);
>>>> +    put_device(core->dev);
>>>> +    kfree(core);
>>>> +}
>>>> +
>>> What you would have here is an admixture of devm allocated stuff and
>>> hand-allocated, refcounted with custom v4l2_release() which might all work
>>> but, it's not obvious to me does work i.e. core is hand allocated while
>>> reg_base, response_packet etc are devm() managed.
>> reg_base and response_packet are safe: after iris_remove() the IRQ is
>> freed and the core is in IRIS_CORE_DEINIT, so nothing uses them. Only
>> the core structure must outlive remove(), so only that is refcounted.
>
> Right and you can confidently assert that for clocks, regulators, power
> domains and ICC ?

Clocks, power domains and ICC are only used in the power on/off
sequence and the clock/ICC votes. iris_core_deinit() powers the
hardware off in remove(). Will handle this more precisely in the
next revision.

>
> TBH I think this is pretty hacky and unlikely to be safe.
>
> Another way I don't think this approach works is an ioctl() waiting on
> firmware do do whatever. If the ioctl wakes up after the devres() removal -
> then what ?

The wait times out and returns an error. That return path does
not access the freed resources.

>
>>> A much simpler solution to your problem is :
>>>
>>> static struct platform_driver iris_driver = {
>>>      .driver = {
>>>          .suppress_bind_attrs = true,
>>>          ...
>>>      },
>>> };
>>>
>> This was discussed in [1]. Dmitry noted it is a sign of a badly written driver,
>> and I agree: it removes the unbind interface to avoid the problem instead of
>> fixing remove().
>>
>> [1]
>> https://lore.kernel.org/all/
>> CAO9ioeUvV+3sME9P1bJTpDOSty=uojOKak659XCf337++vPVNg@mail.gmail.com
>
> In which case I think you need to do more poving/reasoning here about
> admixture of devm and hand-rolled allocations.

Agreed, will explain more in the next revision which resources are
devm, which one outlives remove(), and why the others are not
used after it.

>
> Like what's the use-case you are supporting here with the unbind exactly ?

Unbind through sysfs is generic driver-core behavior, and remove()
should be safe whenever it is called.

This series started from the issue reported by sashiko in [1]. Will add reported
by and closes: link to it in the next revision.

[1] https://lore.kernel.org/all/20260926064921.976721F000FF@smtp.kernel.org

>
> ---
> bod