On Sat, 19 Mar 2016 14:58:21 +0100,
Takashi Sakamoto wrote:
>
> 'struct snd_timer_gparams' includes some members with 'unsigned long',
> therefore its size differs depending on data models (ILP32/LP64). As a
> result, x86/x32 applications fail to execute ioctl(2) with
> SNDRV_TIMER_GPARAMS on x86_64 machine.
>
> This commit fixes this bug by adding a pair of structure and ioctl
> command for the compatibility.
>
> Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
> ---
> sound/core/timer_compat.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
> index 2e90822..5809387 100644
> --- a/sound/core/timer_compat.c
> +++ b/sound/core/timer_compat.c
> @@ -22,6 +22,18 @@
>
> #include <linux/compat.h>
>
> +/*
> + * In LP64, 64 bit storage alignment is used, therefore the size of this
> + * structure is expanded to multiple of 8.
You can't say the 8 bytes alignment as LP64 generic. It rather
depends on each architecture. Write something like "64bit storage
alignment may be used (e.g. x86-64)".
> But the size should be aligned to
> + * multiple of 4 for ILP32.
Actually, this isn't guaranteed so but practically seen, all 32bit
architectures on Linux are at most 4 bytes alignment.
> This is a reason to use 'packed' attribute.
> + */
> +struct snd_timer_gparams32 {
> + struct snd_timer_id tid;
> + u32 period_num;
> + u32 period_den;
> + unsigned char reserved[32];
> +}__attribute__((packed));
> +
> struct snd_timer_info32 {
> u32 flags;
> s32 card;
> @@ -32,6 +44,19 @@ struct snd_timer_info32 {
> unsigned char reserved[64];
> };
>
> +static int snd_timer_user_gparams_compat(struct file *file,
> + struct snd_timer_gparams32 __user *user)
> +{
> + struct snd_timer_gparams gparams;
> +
> + if (copy_from_user(&gparams, user,
> + sizeof(struct snd_timer_id) + sizeof(u32) + sizeof(u32)))
> + return -EFAULT;
> + return snd_timer_user_gparams(file,
> + (struct snd_timer_gparams __user *)&gparams);
This is not correct in two ways:
- You didn't convert the data to the 64bit struct. So the passed
values are garbled.
- Passing the instance on the stack as a user pointer is broken
(unless you do some hacks).
For the first, you'd need to copy each field one by one. For the
latter, snd_timer_user_gparams() needs to be split to two version, one
for kernel pointer and one for user pointer with copy_from_user().
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
