[PATCH 6/7] drm/rcar: gem: dumb: pitch is an output

[Laurent Pinchart]

Hi Thierry,

Thank you for the patch.

On Wednesday 05 November 2014 14:25:18 Thierry Reding wrote:
> From: Thierry Reding <treding at nvidia.com>
> 
> When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB
> IOCTL, only the width, height, bpp and flags fields are inputs. The
> caller is not guaranteed to zero out or set handle, pitch and size.
> Drivers must not treat these values as possible inputs, otherwise they
> may use uninitialized memory during the computation of the framebuffer
> size.
> 
> The R-Car DU driver treats the pitch passed in from userspace as minimum
> and will only overwrite it when the driver-computed pitch is larger,
> allowing userspace to, intentionally or not, overallocate framebuffers.

As discussed on IRC, my concern with this is that some userspace applications 
might be relying on this behaviour. I'm not personally aware of any though, so 
I'm not opposed to this patch, but I can't vouch it won't cause any userspace 
breakage.

(On a side note I believe treating the pitch and size arguments as inputs 
could be a worthwhile extension to the API, but given that we haven't rejected 
incorrect values in the past we're pretty much stuck).

> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 5329491e32c3..6289e3797bc5
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -126,7 +126,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct
> drm_device *dev, else
>  		align = 16 * args->bpp / 8;
> 
> -	args->pitch = roundup(max(args->pitch, min_pitch), align);
> +	args->pitch = roundup(min_pitch, align);
> 
>  	return drm_gem_cma_dumb_create_internal(file, dev, args);
>  }

-- 
Regards,

Laurent Pinchart