On Mon, Mar 30, 2020 at 12:29:44PM +0200, Sam Ravnborg wrote:
> On Sat, Mar 28, 2020 at 11:02:26PM +0100, Daniel Vetter wrote:
> > On Sat, Mar 28, 2020 at 7:49 PM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote:
> > >
> > > Hi Daniel.
> > >
> > > On Sat, Mar 28, 2020 at 05:23:58PM +0100, Daniel Vetter wrote:
> > > > I'm thinking this is the warning that fired in the 0day report, but I
> > > > can't double-check yet since 0day didn't upload its source tree
> > > > anywhere I can check. And all the drivers I can easily test don't use
> > > > drm_dev_alloc anymore ...
> > > >
> > > > Also if I'm correct supreme amounts of bad luck because usually kslap
> > > > (for bigger structures) gives us something quite a bit bigger than
> > > > what we asked for.
> > > >
> > > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > > Fixes: c6603c740e0e ("drm: add managed resources tied to drm_device")
> > > > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > > Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > > Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx
> > > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/drm_managed.c | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> > > > index 4955241ceb4c..9cebfe370a65 100644
> > > > --- a/drivers/gpu/drm/drm_managed.c
> > > > +++ b/drivers/gpu/drm/drm_managed.c
> > > > @@ -139,8 +139,7 @@ void drmm_add_final_kfree(struct drm_device *dev, void *container)
> > > > {
> > > > WARN_ON(dev->managed.final_kfree);
> > > > WARN_ON(dev < (struct drm_device *) container);
> > > > - WARN_ON(dev + 1 >=
> > > > - (struct drm_device *) (container + ksize(container)));
> > > > + WARN_ON(dev + 1 > (struct drm_device *) (container + ksize(container)));
> > >
> > > I do not think this is the right fix...
> > > The original code would trigger if
> > > 1) the container only had a drm_device - and nothing else
> > > 2) and the allocated size was the same
> >
> > Yup, which apparently happens for all the drivers calling
> > drm_dev_alloc(). At least on the unlucky architecture that 0day tested
> > on (or build settings, or whatever). The issue was hit with drm/bochs,
> > which is still using drm_dev_alloc (like most older-ish drivers).
>
> That explains it and then the checks makes sense.
>
> Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
Thanks for your review, patch applied and a note to the commit message
added that 0day confirmed that it's indeed this WARN_ON that they've hit.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
