linux-kernel - Re: [PATCH 2/4] drm/msm: rework GEM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a2ePkk21bz=N=97aMGJVoxSu-cULjLZbf03W4jhSQAGJA@mail.gmail.com>
Date:   Fri, 30 Nov 2018 16:14:30 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Rob Clark <robdclark@...il.com>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        Jordan Crouse <jcrouse@...eaurora.org>,
        David Airlie <airlied@...ux.ie>, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] drm/msm: rework GEM_INFO ioctl

On Fri, Nov 30, 2018 at 4:02 PM Rob Clark <robdclark@...il.com> 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