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@sakamocchi.jp

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(&register_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(&register_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