[PATCH] drm/managed: Fix off-by-one in warning

[Daniel Vetter]

On Sat, Mar 28, 2020 at 7:49 PM Sam Ravnborg <sam at ravnborg.org> 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 at intel.com>
> > Fixes: c6603c740e0e ("drm: add managed resources tied to drm_device")
> > Cc: Sam Ravnborg <sam at ravnborg.org>
> > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > Cc: Dan Carpenter <dan.carpenter at oracle.com>
> > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Cc: Neil Armstrong <narmstrong at baylibre.com
> > Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> > Cc: "Rafael J. Wysocki" <rafael at kernel.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  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).

> And the modification will now allow for a container with the exact size
> of drm_device.
>
> I checked all users in my tree - no-one only had a drm_device.
> The minimum was one extra pointer.
>
> Another thing that could trigger the warning was if any users
> did not specify a pointer to memory allocated by k(z)alloc()
> But I could not find any.
>
> tiny/st7735r.c looked suspisius, but I think it is also OK,
> because struct st7735r_priv is allocated, but the poitner specified in
> st7735r_priv.dbidev. But dbidev is the first field - so OK.
>
> So no better clue...

Yeah all the drivers using drm_dev_init with embedded drm_device wont hit this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch