On Wed, Mar 26, 2025 at 10:23:04AM +0100, Luca Ceresoli wrote: > On Tue, 25 Mar 2025 13:24:09 -0400 > Anusha Srivatsa <asriv...@redhat.com> wrote: > > > Allocate panel via reference counting. > > Add _get() and _put() helper functions > > to ensure panel allocations are refcounted. > > Avoid use after free by ensuring panel is > > valid and can be usable till the last reference > > is put. This avoids use-after-free > > "panel is valid and can be usable" is not totally correct. When there > are still references held, you ensure the panel struct is still > _allocated_, not necessarily valid and usable.
I guess "panel pointer is valid" is a better wording indeed.
> > +/**
> > + * drm_panel_put - Release a panel reference
> > + * @panel: DRM panel
> > + *
> > + * This function decrements the panel's reference count and frees the
> > + * object if the reference count drops to zero.
> > + */
> > +struct drm_panel *drm_panel_put(struct drm_panel *panel)
> > +{
> > + if (!panel)
> > + return panel;
> > +
> > + kref_put(&panel->refcount, __drm_panel_free);
> > +
> > + return panel;
>
> While this is using the same structure as my bridge work, I now realize
> the _put() should probably not return the panel (or bridge) pointer, it
> should just be a void function. The reason is the pointer might have
> been freed when _put() returns (depending on the refcount) so that
> pointer value might be dangling and whoever calls _put() must not use
> that pointer anymore.
>
> Other get/put APIs do this, e.g. of_node_get/put().
>
> So I'm going to change drm_bridge_put() to return void, unless there
> are good reasons to keep it and that I'm missing.
Oh, right, definitely.
Maxime
signature.asc
Description: PGP signature
