Hi,
On Mar 23 2016 00:11, Takashi Iwai wrote:
> On Tue, 22 Mar 2016 16:04:06 +0100,
> Takashi Sakamoto wrote:
>>
>> 'struct snd_timer_gparams' includes some members with 'unsigned long',
>> therefore its size differs depending on data models of architecture. As
>> a result, x86/x32 applications fail to execute ioctl(2) with
>> SNDRV_TIMER_GPARAMS command 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.c | 20 +++++++++++++-------
>> sound/core/timer_compat.c | 34 +++++++++++++++++++++++++++++++++-
>> 2 files changed, 46 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/core/timer.c b/sound/core/timer.c
>> index aa1b15c..ea4d999 100644
>> --- a/sound/core/timer.c
>> +++ b/sound/core/timer.c
>> @@ -1502,17 +1502,13 @@ static int snd_timer_user_ginfo(struct file *file,
>> return err;
>> }
>>
>> -static int snd_timer_user_gparams(struct file *file,
>> - struct snd_timer_gparams __user *_gparams)
>> +static int timer_set_gparams(struct snd_timer_gparams *gparams)
>> {
>> - struct snd_timer_gparams gparams;
>> struct snd_timer *t;
>> int err;
>>
>> - if (copy_from_user(&gparams, _gparams, sizeof(gparams)))
>> - return -EFAULT;
>> mutex_lock(®ister_mutex);
>> - t = snd_timer_find(&gparams.tid);
>> + t = snd_timer_find(&gparams->tid);
>> if (!t) {
>> err = -ENODEV;
>> goto _error;
>> @@ -1525,12 +1521,22 @@ static int snd_timer_user_gparams(struct file *file,
>> err = -ENOSYS;
>> goto _error;
>> }
>> - err = t->hw.set_period(t, gparams.period_num, gparams.period_den);
>> + err = t->hw.set_period(t, gparams->period_num, gparams->period_den);
>> _error:
>> mutex_unlock(®ister_mutex);
>> return err;
>> }
>>
>> +static int snd_timer_user_gparams(struct file *file,
>> + struct snd_timer_gparams __user *_gparams)
>> +{
>> + struct snd_timer_gparams gparams;
>> +
>> + if (copy_from_user(&gparams, _gparams, sizeof(gparams)))
>> + return -EFAULT;
>> + return timer_set_gparams(&gparams);
>> +}
>> +
>> static int snd_timer_user_gstatus(struct file *file,
>> struct snd_timer_gstatus __user *_gstatus)
>> {
>> diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
>> index 2e90822..d10c792 100644
>> --- a/sound/core/timer_compat.c
>> +++ b/sound/core/timer_compat.c
>> @@ -22,6 +22,19 @@
>>
>> #include <linux/compat.h>
>>
>> +/*
>> + * ILP32/LP64 has different size for 'long' type. Additionally, the size
>> + * of storage alignment differs depending on architectures. Here, '__packed'
>> + * qualifier is used so that the size of this structure is multiple of 4 and
>> + * it fits to any architectures with 32 bit storage alignment.
>> + */
>> +struct snd_timer_gparams32 {
>> + struct snd_timer_id tid;
>> + u32 period_num;
>> + u32 period_den;
>> + unsigned char reserved[32];
>> +} __packed;
>> +
>> struct snd_timer_info32 {
>> u32 flags;
>> s32 card;
>> @@ -32,6 +45,23 @@ 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 (get_user(gparams.tid.dev_class, &user->tid.dev_class) ||
>> + get_user(gparams.tid.dev_sclass, &user->tid.dev_sclass) ||
>> + get_user(gparams.tid.card, &user->tid.card) ||
>> + get_user(gparams.tid.device, &user->tid.device) ||
>> + get_user(gparams.tid.subdevice, &user->tid.subdevice) ||
>
> struct snd_timer_id is compatible, so you can use just
> copy_from_user() for the whole tid, while the rest two fields need to
> be copied individually.
>
> Other than that, the patch looks good.
OK. I'll post revised version (v3), soon.
Regards
Takashi Sakamoto
> thanks,
>
> Takashi
>
>
>> + get_user(gparams.period_num, &user->period_num) ||
>> + get_user(gparams.period_den, &user->period_den))
>> + return -EFAULT;
>> +
>> + return timer_set_gparams(&gparams);
>> +}
>> +
>> static int snd_timer_user_info_compat(struct file *file,
>> struct snd_timer_info32 __user *_info)
>> {
>> @@ -99,6 +129,7 @@ static int snd_timer_user_status_compat(struct file *file,
>> */
>>
>> enum {
>> + SNDRV_TIMER_IOCTL_GPARAMS32 = _IOW('T', 0x04, struct snd_timer_gparams32),
>> SNDRV_TIMER_IOCTL_INFO32 = _IOR('T', 0x11, struct snd_timer_info32),
>> SNDRV_TIMER_IOCTL_STATUS32 = _IOW('T', 0x14, struct snd_timer_status32),
>> #ifdef CONFIG_X86_X32
>> @@ -114,7 +145,6 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
>> case SNDRV_TIMER_IOCTL_PVERSION:
>> case SNDRV_TIMER_IOCTL_TREAD:
>> case SNDRV_TIMER_IOCTL_GINFO:
>> - case SNDRV_TIMER_IOCTL_GPARAMS:
>> case SNDRV_TIMER_IOCTL_GSTATUS:
>> case SNDRV_TIMER_IOCTL_SELECT:
>> case SNDRV_TIMER_IOCTL_PARAMS:
>> @@ -128,6 +158,8 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
>> case SNDRV_TIMER_IOCTL_PAUSE_OLD:
>> case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
>> return snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
>> + case SNDRV_TIMER_IOCTL_GPARAMS32:
>> + return snd_timer_user_gparams_compat(file, argp);
>> case SNDRV_TIMER_IOCTL_INFO32:
>> return snd_timer_user_info_compat(file, argp);
>> case SNDRV_TIMER_IOCTL_STATUS32:
>> --
>> 2.7.3
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
