On Mon, Jun 08, 2026 at 05:41:11PM +0200, Romain Gantois wrote:
> Hi Maxime,
>
> On Monday, 1 June 2026 09:11:21 CEST Maxime Ripard wrote:
> > Hi,
> >
> > On Mon, Jun 01, 2026 at 08:52:44AM +0200, Romain Gantois wrote:
> > > The logicvc driver calls drm_universal_plane_init(),
> > > drm_crtc_init_with_planes(), and drm_encoder_alloc(). These functions
> > > should not be called with structs allocated with devm_kzalloc(), as this
> > > can lead to use-after-free bugs. In fact, a use-after-free caused by this
> > > has been observed on a v6.6 kernel.
> > >
> > > Use DRM-managed allocations instead for panel, CRTC and encoder objects.
> > >
> > > Found using KASAN.
> > >
> > > Fixes: efeeaefe9be56 ("drm: Add support for the LogiCVC display
> > > controller") Cc: [email protected]
> > > Signed-off-by: Romain Gantois <[email protected]>
> >
> > You're only partially fixing the issue. You also need to protect any
> > device resource (register mapping, clocks, etc) are no longer accessed
> > after the device has been removed, and this is typically done using
> > drm_dev_enter/exit.
>
> Sorry there's something which I don't quite understand: is this a new issue
> which is specifically introduced by my changes in this series, or a different
> issue in this driver which isn't handled by my series?A bit of both I guess ? :) My point was that while your commit log claims you avoid use-after-free, and your patch definitely avoids some, you can still trivially trigger some. Whether you want to fix them all at once or prefer to defer it to a later point in time is equally fine by me, but you need to be aware that it's not done, and you probably want to have it in the commit log somewhere too? > IIUC all I'm doing here is just letting the drmm code handle cleaning up the > plane, crtc, etc. objects instead of doing it "by hand" with devm_kzalloc. > Why > does this make it necessary to add additional protection of driver resources? It's not necessary, but it's also kind of the same issue. The reason we need to have drmm over devm is that the driver stays around longer than its device, so devm-allocated memory would have been freed. But that's also the case for *any* devm resource, or more generally any resource linked to that device, so register mappings, clocks, interrupts, etc. So yeah, it's a different symptom of the same underlying cause. Maxime
signature.asc
Description: PGP signature
