Re: [PATCH v5 1/8] drm/msm/dpu: don't mix devm and drmm functions

[Rob Clark]

On Thu, May 7, 2026 at 9:29 AM John Harrison <John.Harrison@xxxxxxxxxx> wrote:
>
> Resending because apparently it got sent as HTML and was rejected by the
> mailing lists...
>
> On 5/5/26 14:49, Rob Clark wrote:
> > On Mon, May 4, 2026 at 5:25 PM Dmitry Baryshkov
> > <dmitry.baryshkov@xxxxxxxxxxxxxxxx> wrote:
> >> Mixing devm and drmm functions will result in a use-after-free on msm
> >> driver teardown if userspace keeps a reference on the drm device:
> >> The WB connector data will be destroyed because of the use of
> >> devm_kzalloc()), while the usersoace still can try interacting with the
> >> WB connector (which uses drmm_ functions).
> >>
> >> Change dpu_writeback_init() to use drmm_.
> >  From [1] it doesn't sound like userspace holding the drm device open
> > is the issue (if that were possible, it seems like it would go badly),
> > but rather the order of managed cleanup?
> >
> > [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/xe/xe_hwmon.c?id=3a13c2de442d6bfaef9c102cd1092e6cae22b753
> So is this not an actual issue that has been seen in the wild? It is
> just a theoretical issue based on code observation?
>
> If so, then maybe the comment should just be something like:
>
>     dpu_writeback_init() was mixing devm and drmm functions - allocating
>     using devm and then passing to drmm to manage. This creates the
>     potential for a use-after-free bug as drmm and devm have different
>     lifetimes. Fix that by consistently using drmm management.
>

I've not seen this issue myself, but I guess Dmitry has.  That comment
sounds more in-line with what I _think_ is happening..

BR,
-R

>
> John.
>
> >
> >> Fixes: 0b37ac63fc9d ("drm/msm/dpu: use drmm_writeback_connector_init()")
> >> Reported-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> >> Closes: https://lore.kernel.org/r/78c764b8-44cf-4db5-88e7-807a85954518@xxxxxxxxxx
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> >> ---
> >>   drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> >> index 7545c0293efb..6f2370c9dd98 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> >> @@ -5,6 +5,7 @@
> >>
> >>   #include <drm/drm_edid.h>
> >>   #include <drm/drm_framebuffer.h>
> >> +#include <drm/drm_managed.h>
> >>
> >>   #include "dpu_writeback.h"
> >>
> >> @@ -125,7 +126,7 @@ int dpu_writeback_init(struct drm_device *dev, struct drm_encoder *enc,
> >>          struct dpu_wb_connector *dpu_wb_conn;
> >>          int rc = 0;
> >>
> >> -       dpu_wb_conn = devm_kzalloc(dev->dev, sizeof(*dpu_wb_conn), GFP_KERNEL);
> >> +       dpu_wb_conn = drmm_kzalloc(dev, sizeof(*dpu_wb_conn), GFP_KERNEL);
> >>          if (!dpu_wb_conn)
> >>                  return -ENOMEM;
> >>
> >>
> >> --
> >> 2.47.3
> >>
>