[PATCH v2] drm: Fix alignment of temporary stack ioctl buffers

[T.J. Mercier]

On Fri, Jun 14, 2024 at 4:47 AM <carsten.haitzler at foss.arm.com> wrote:
>
> From: Carsten Haitzler <carsten.haitzler at foss.arm.com>
>
> In a few places (core drm + AMD kfd driver), the ioctl handling uses a
> temporary 128 byte buffer on the stack to copy to/from user. ioctl data
> can have structs with types of much larger sizes than a byte and a
> system may require alignment of types in these. At the same time the
> compiler may align a char buf to something else as it has no idea that
> this buffer is used for storing structs with such alignment
> requirements. At a minimum putting in alignment information as an
> attribute should be a warning in future if an architecture that needs
> more alignment appears.
>
> This was discovered while implementing capability ABI support in Linux
> on ARM's Morello CPU (128 bit capability "pointers" in userspace, with
> a 64bit non-capability kernel (hybrid) setup). In this, userspace
> ioctl structs now had to transport capabilities that needed 16 byte
> alignment, but the kernel was not putting these data buffers on that
> alignment boundary.
>
> Currently the largest type that is needed is a u64 so the alignment
> only asks for that.
>
> Signed-off-by: Carsten Haitzler <carsten.haitzler at foss.arm.com>

Reviewed-by: T.J. Mercier <tjmercier at google.com>

> ---
>  drivers/dma-buf/dma-heap.c               | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
>  drivers/gpu/drm/drm_ioctl.c              | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 84ae708fafe7..8fa68b8a9b60 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -126,7 +126,7 @@ static unsigned int dma_heap_ioctl_cmds[] = {
>  static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
>                            unsigned long arg)
>  {
> -       char stack_kdata[128];
> +       _Alignas(u64) char stack_kdata[128];
>         char *kdata = stack_kdata;
>         unsigned int kcmd;
>         unsigned int in_size, out_size, drv_size, ksize;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index fdf171ad4a3c..201a5c0227ec 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -3236,7 +3236,7 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>         amdkfd_ioctl_t *func;
>         const struct amdkfd_ioctl_desc *ioctl = NULL;
>         unsigned int nr = _IOC_NR(cmd);
> -       char stack_kdata[128];
> +       _Alignas(u64) char stack_kdata[128];
>         char *kdata = NULL;
>         unsigned int usize, asize;
>         int retcode = -EINVAL;
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index e368fc084c77..77a88b597c0b 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -767,7 +767,7 @@ long drm_ioctl(struct file *filp,
>         drm_ioctl_t *func;
>         unsigned int nr = DRM_IOCTL_NR(cmd);
>         int retcode = -EINVAL;
> -       char stack_kdata[128];
> +       _Alignas(u64) char stack_kdata[128];
>         char *kdata = NULL;
>         unsigned int in_size, out_size, drv_size, ksize;
>         bool is_driver_ioctl;
> --
> 2.25.1
>