Re: [PATCH] drm: Reject unknown legacy bpp and dpeth for drm_mode_addfb ioctl

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: Reject unknown legacy bpp and dpeth for drm_mode_addfb ioctl
Date: Tue, 4 Sep 2018 23:46:33 +0200	[thread overview]
Message-ID: <20180904214633.GU21634@phenom.ffwll.local> (raw)
In-Reply-To: <20180904205319.11174-1-chris@chris-wilson.co.uk>

On Tue, Sep 04, 2018 at 09:53:19PM +0100, Chris Wilson wrote:
> Since this is handling user provided bpp and depth, we need to sanity
> check and propagate the EINVAL back rather than assume what the insane
> client intended and fill the logs with DRM_ERROR.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> So I am presuming that r.pixel_format == 0 is rejected elsewhere for the
> internal users (as if any would deliberately provoke the error)!

Could maybe add a DRM_FORMAT_INVALID at the end of drm_fourcc.h, and then
switch over the various format/modifier tables to being zero terminated.
Well DRM_FORMAT_MOD_INVALID can't be 0 because that means linear. Anyway,
I digress, this loks good.

And yes drm_internal_framebuffer_create makes sure you have a real fourcc
code, not a figment of your imagination (or more profane, stack garbage).

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Same as with the previous one, igt would be sweet on top.
-Daniel

> ---
>  drivers/gpu/drm/drm_fourcc.c      | 4 ++--
>  drivers/gpu/drm/drm_framebuffer.c | 5 ++++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 35c1e2742c27..34595c5b55c9 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -69,8 +69,8 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>  			fmt = DRM_FORMAT_ARGB8888;
>  		break;
>  	default:
> -		DRM_ERROR("bad bpp, assuming x8r8g8b8 pixel format\n");
> -		fmt = DRM_FORMAT_XRGB8888;
> +		DRM_DEBUG("bad bpp [%d] and depth [%d]\n", bpp, depth);
> +		fmt = 0;
>  		break;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 781af1d42d76..7641bddfe367 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -112,12 +112,15 @@ int drm_mode_addfb(struct drm_device *dev, struct drm_mode_fb_cmd *or,
>  	struct drm_mode_fb_cmd2 r = {};
>  	int ret;
>  
> +	r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
> +	if (!r.pixel_format)
> +		return -EINVAL;
> +
>  	/* convert to new format and call new ioctl */
>  	r.fb_id = or->fb_id;
>  	r.width = or->width;
>  	r.height = or->height;
>  	r.pitches[0] = or->pitch;
> -	r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
>  	r.handles[0] = or->handle;
>  
>  	if (r.pixel_format == DRM_FORMAT_XRGB2101010 &&
> -- 
> 2.19.0.rc1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx