[PATCH 08/15] drm: omapdrm: gem: Mask out private flags passed from userspace

[Emil Velikov]

On 4 December 2015 at 22:27, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
> The 8 high order bits of the buffer flags are reserved for internal use.
> Mask them out from the flags passed by userspace.
>
Ouch... I know that the Intel guys are pretty rigorous about input
validation, although I wonder how many drivers are (partially or not)
missing these.

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 9 ++++++---
>  include/uapi/drm/omap_drm.h        | 1 +
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 83d63d71062c..e405ab23d7a6 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -551,10 +551,13 @@ static int ioctl_gem_new(struct drm_device *dev, void *data,
>                 struct drm_file *file_priv)
>  {
>         struct drm_omap_gem_new *args = data;
> +       u32 flags = args->flags & OMAP_BO_USER_MASK;
> +
>         VERB("%p:%p: size=0x%08x, flags=%08x", dev, file_priv,
> -                       args->size.bytes, args->flags);
> -       return omap_gem_new_handle(dev, file_priv, args->size,
> -                       args->flags, &args->handle);
> +            args->size.bytes, flags);
> +
> +       return omap_gem_new_handle(dev, file_priv, args->size, flags,
> +                                  &args->handle);
>  }
>
>  static int ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
> diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
> index 1d0b1172664e..6852c26e8f78 100644
> --- a/include/uapi/drm/omap_drm.h
> +++ b/include/uapi/drm/omap_drm.h
> @@ -36,6 +36,7 @@ struct drm_omap_param {
>  #define OMAP_BO_SCANOUT                0x00000001      /* scanout capable (phys contiguous) */
>  #define OMAP_BO_CACHE_MASK     0x00000006      /* cache type mask, see cache modes */
>  #define OMAP_BO_TILED_MASK     0x00000f00      /* tiled mapping mask, see tiled modes */
> +#define OMAP_BO_USER_MASK      0x00ffffff      /* flags settable by userspace */
>
Fwiw I'm not too sure that adding the mask here  (UAPI) is a good
idea. If you like to keep it, I'd suggest that it encapsulates only
what's currently available. Might even want to move the individual
masks in the respective sections/hunks. Speaking of which  -
OMAP_BO_TILED_MASK should be 0x00000300, shouldn't it ?

Regards,
Emil