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

Daniel Vetter
2018-09-04T21:46:37Z

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