stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gem: Detect overflow in calculating dumb buffer size
@ 2020-01-23 12:59 Chris Wilson
  2020-01-23 13:27 ` [Intel-gfx] " Ville Syrjälä
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2020-01-23 12:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Ramalingam C, Joonas Lahtinen, stable

To multiply 2 u32 numbers to generate a u64 in C requires a bit of
forewarning for the compiler.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0a20083321a3..ff79da5657f8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -265,7 +265,10 @@ i915_gem_dumb_create(struct drm_file *file,
 						    DRM_FORMAT_MOD_LINEAR))
 		args->pitch = ALIGN(args->pitch, 4096);
 
-	args->size = args->pitch * args->height;
+	if (args->pitch < args->width)
+		return -EINVAL;
+
+	args->size = mul_u32_u32(args->pitch, args->height);
 
 	mem_type = INTEL_MEMORY_SYSTEM;
 	if (HAS_LMEM(to_i915(dev)))
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/gem: Detect overflow in calculating dumb buffer size
  2020-01-23 12:59 [PATCH] drm/i915/gem: Detect overflow in calculating dumb buffer size Chris Wilson
@ 2020-01-23 13:27 ` Ville Syrjälä
  2020-01-23 13:39   ` Chris Wilson
  0 siblings, 1 reply; 4+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, stable

On Thu, Jan 23, 2020 at 12:59:34PM +0000, Chris Wilson wrote:
> To multiply 2 u32 numbers to generate a u64 in C requires a bit of
> forewarning for the compiler.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0a20083321a3..ff79da5657f8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -265,7 +265,10 @@ i915_gem_dumb_create(struct drm_file *file,
>  						    DRM_FORMAT_MOD_LINEAR))
>  		args->pitch = ALIGN(args->pitch, 4096);
>  
> -	args->size = args->pitch * args->height;
> +	if (args->pitch < args->width)
> +		return -EINVAL;
> +
> +	args->size = mul_u32_u32(args->pitch, args->height);

I thought something would have checked these against the mode_config
fb limits already. But can't see code like that anywhere. Maybe we
should just do that in the core?

>  
>  	mem_type = INTEL_MEMORY_SYSTEM;
>  	if (HAS_LMEM(to_i915(dev)))
> -- 
> 2.25.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/gem: Detect overflow in calculating dumb buffer size
  2020-01-23 13:27 ` [Intel-gfx] " Ville Syrjälä
@ 2020-01-23 13:39   ` Chris Wilson
  2020-01-23 13:58     ` Ville Syrjälä
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2020-01-23 13:39 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, stable

Quoting Ville Syrjälä (2020-01-23 13:27:07)
> On Thu, Jan 23, 2020 at 12:59:34PM +0000, Chris Wilson wrote:
> > To multiply 2 u32 numbers to generate a u64 in C requires a bit of
> > forewarning for the compiler.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ramalingam C <ramalingam.c@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 0a20083321a3..ff79da5657f8 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -265,7 +265,10 @@ i915_gem_dumb_create(struct drm_file *file,
> >                                                   DRM_FORMAT_MOD_LINEAR))
> >               args->pitch = ALIGN(args->pitch, 4096);
> >  
> > -     args->size = args->pitch * args->height;
> > +     if (args->pitch < args->width)
> > +             return -EINVAL;
> > +
> > +     args->size = mul_u32_u32(args->pitch, args->height);
> 
> I thought something would have checked these against the mode_config
> fb limits already. But can't see code like that anywhere. Maybe we
> should just do that in the core?

While it is in uapi/drm_mode.h, is there any restriction that the dumb
buffer has to be used with a framebuffer? Not that I have a good use
case, just wondering if we need to be so proscriptive.

We create something that is compatible but presume we will need later
validation against HW.
-Chris

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/gem: Detect overflow in calculating dumb buffer size
  2020-01-23 13:39   ` Chris Wilson
@ 2020-01-23 13:58     ` Ville Syrjälä
  0 siblings, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, stable

On Thu, Jan 23, 2020 at 01:39:03PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjälä (2020-01-23 13:27:07)
> > On Thu, Jan 23, 2020 at 12:59:34PM +0000, Chris Wilson wrote:
> > > To multiply 2 u32 numbers to generate a u64 in C requires a bit of
> > > forewarning for the compiler.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Ramalingam C <ramalingam.c@intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 0a20083321a3..ff79da5657f8 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -265,7 +265,10 @@ i915_gem_dumb_create(struct drm_file *file,
> > >                                                   DRM_FORMAT_MOD_LINEAR))
> > >               args->pitch = ALIGN(args->pitch, 4096);
> > >  
> > > -     args->size = args->pitch * args->height;
> > > +     if (args->pitch < args->width)
> > > +             return -EINVAL;
> > > +
> > > +     args->size = mul_u32_u32(args->pitch, args->height);
> > 
> > I thought something would have checked these against the mode_config
> > fb limits already. But can't see code like that anywhere. Maybe we
> > should just do that in the core?
> 
> While it is in uapi/drm_mode.h, is there any restriction that the dumb
> buffer has to be used with a framebuffer? Not that I have a good use
> case, just wondering if we need to be so proscriptive.

I think the general concensus has been that anything else is an abuse
of the interface (not that it has stopped people from doing it IIRC).

But maybe there's some good use for it that I can't think up.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> We create something that is compatible but presume we will need later
> validation against HW.
> -Chris

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-23 13:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 12:59 [PATCH] drm/i915/gem: Detect overflow in calculating dumb buffer size Chris Wilson
2020-01-23 13:27 ` [Intel-gfx] " Ville Syrjälä
2020-01-23 13:39   ` Chris Wilson
2020-01-23 13:58     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).