Re: [PATCH 2/4] drm/msm: rework GEM_INFO ioctl

[Arnd Bergmann]

On Fri, Nov 30, 2018 at 4:02 PM Rob Clark <robdclark@xxxxxxxxx> wrote:
>

> -
> -#define MSM_INFO_FLAGS (MSM_INFO_IOVA)
> +/* Get or set GEM buffer info.  The requested value can be passed
> + * directly in 'value', or for data larger than 64b 'value' is a
> + * pointer to userspace buffer, with 'len' specifying the number of
> + * bytes copied into that buffer.  For info returned by pointer,
> + * calling the GEM_INFO ioctl with null 'value' will return the
> + * required buffer size in 'len'
> + */
> +#define MSM_INFO_GET_OFFSET    0x00   /* get mmap() offset, returned by value */
> +#define MSM_INFO_GET_IOVA      0x01   /* get iova, returned by value */
>
>  struct drm_msm_gem_info {
>         __u32 handle;         /* in */
> -       __u32 flags;          /* in - combination of MSM_INFO_* flags */
> -       __u64 offset;         /* out, mmap() offset or iova */
> +       __u32 info;           /* in - one of MSM_INFO_* */
> +       __u64 value;          /* in or out */
> +       __u32 len;            /* in or out */
>  };

As structure with implicit padding has the problem of possibly leaking
kernel stack data. It's better to make the padding explicit here so you
can zero it from the kernel. Also, as I mentioned in the other patch,
you probably need a new data structure and ioctl command number
to keep compatiblity with the old interface.

     Arnd