On Tue, Jun 11, 2024 at 2:35 AM <carsten.haitzler@xxxxxxxxxxxx> wrote:
>
> From: Carsten Haitzler <carsten.haitzler@xxxxxxxxxxxx>
>
> 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.
Makes sense to me.
Now that the kernel depends on C11, I think:
__attribute__((aligned(__alignof__(u64))))
can be simply reduced to:
_Alignas(u64)
and put first instead of last in the declaration:
_Alignas(u64) char stack_kdata[128];
>
> Signed-off-by: Carsten Haitzler <carsten.haitzler@xxxxxxxxxxxx>
> ---
> 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..a49e20440edf 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];
> + char stack_kdata[128] __attribute__((aligned(__alignof__(u64))));
> 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..69a0aae0f016 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];
> + char stack_kdata[128] __attribute__((aligned(__alignof__(u64))));
> 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..aac3d5a539a6 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];
> + char stack_kdata[128] __attribute__((aligned(__alignof__(u64))));
> char *kdata = NULL;
> unsigned int in_size, out_size, drv_size, ksize;
> bool is_driver_ioctl;
> --
> 2.25.1
>
