Re: [PATCH v2 5/6] drm/sysfb: simpledrm: Validate mmap size against framebuffer size

[Thierry Reding <treding@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 2727 bytes --]

On Tue, Jun 23, 2026 at 11:02:39AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 22.06.26 um 18:13 schrieb Thierry Reding:
> > On Mon, Jun 22, 2026 at 03:19:39PM +0200, Thomas Zimmermann wrote:
> > > The size of the mmap'ed framebuffer could be smaller than the minimum
> > > required framebuffer size. Validate the resource size against the
> > > framebuffer size.
> > > 
> > > Buggy firmware that triggers this check should be fixed up with a quirk
> > > on a case-by-case base.
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > Suggested-by: Sashiko <sashiko-bot@kernel.org>
> > > ---
> > >   drivers/gpu/drm/sysfb/simpledrm.c | 25 +++++++++++++++++++++++++
> > >   1 file changed, 25 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
> > > index 76b9a3f5c4ef..a04f0c852ea4 100644
> > > --- a/drivers/gpu/drm/sysfb/simpledrm.c
> > > +++ b/drivers/gpu/drm/sysfb/simpledrm.c
> > > @@ -6,6 +6,7 @@
> > >   #include <linux/of_address.h>
> > >   #include <linux/of_clk.h>
> > >   #include <linux/of_reserved_mem.h>
> > > +#include <linux/overflow.h>
> > >   #include <linux/platform_data/simplefb.h>
> > >   #include <linux/platform_device.h>
> > >   #include <linux/pm.h>
> > > @@ -624,6 +625,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> > >   	u16 width_mm = 0, height_mm = 0;
> > >   	struct device_node *panel_node;
> > >   	const struct drm_format_info *format;
> > > +	u64 size;
> > >   	struct resource *res, *mem = NULL;
> > >   	struct drm_plane *primary_plane;
> > >   	struct drm_crtc *crtc;
> > > @@ -704,6 +706,15 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> > >   		}
> > >   		stride = pitch;
> > >   	}
> > > +	if (check_mul_overflow(height, stride, &size)) {
> > > +		drm_err(dev, "framebuffer size exceeds maximum\n");
> > > +		return ERR_PTR(-EINVAL);
> > > +	}
> > > +	size = ALIGN(size, PAGE_SIZE);
> > > +	if (size < PAGE_SIZE) {
> > > +		drm_err(dev, "framebuffer alignment exceeds maximum\n");
> > That error message doesn't make sense to me. Maybe "framebuffer
> > alignment below minimum", or something along those lines?
> 
> This tests that the align operation did not overflow size. The result would
> then be 0. With the earlier patches limiting the height and stride and size
> being of u64, it's not possible any longer, I think. But testing it doesn't
> really cost us much.
> 
> What do you think of "aligned framebuffer size exceeds maximum"?

Ah, I misunderstood what the code was doing. Yes, I think that message
makes it a bit clearer what the error is.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]