| Message ID | 1473763073-17529-1-git-send-email-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 13 Sep 2016 12:37:53 +0200, Takashi Sakamoto wrote: > > When checking value of request for copy operation, current implementation > compares shifted value to macros, while these macros are already shifted. > As a result, it never performs to copy from/to user space. > > This commit fixes the bug. > > Fixes: 8ce8eb601c71('ALSA: seq: add an alternative way to handle ioctl requests' > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> Applied, thanks. Takashi > --- > sound/core/seq/seq_clientmgr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c > index 811b95b..4c93520 100644 > --- a/sound/core/seq/seq_clientmgr.c > +++ b/sound/core/seq/seq_clientmgr.c > @@ -2122,7 +2122,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, > * within 13 bits. We can safely pick up the size from the command. > */ > size = _IOC_SIZE(handler->cmd); > - if (_IOC_DIR(handler->cmd) & IOC_IN) { > + if (handler->cmd & IOC_IN) { > if (copy_from_user(&buf, (const void __user *)arg, size)) > return -EFAULT; > } > @@ -2132,7 +2132,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, > /* Some commands includes a bug in 'dir' field. */ > if (handler->cmd == SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT || > handler->cmd == SNDRV_SEQ_IOCTL_SET_CLIENT_POOL || > - (_IOC_DIR(handler->cmd) & IOC_OUT)) > + (handler->cmd & IOC_OUT)) > if (copy_to_user((void __user *)arg, &buf, size)) > return -EFAULT; > } > -- > 2.7.4 >
On Sep 13 2016 22:47, Takashi Iwai wrote: > On Tue, 13 Sep 2016 12:37:53 +0200, > Takashi Sakamoto wrote: >> >> When checking value of request for copy operation, current implementation >> compares shifted value to macros, while these macros are already shifted. >> As a result, it never performs to copy from/to user space. >> >> This commit fixes the bug. >> >> Fixes: 8ce8eb601c71('ALSA: seq: add an alternative way to handle ioctl requests' >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> > > Applied, thanks. Thanks to apply this, and sorry to have posted with such a critical bug... > Takashi > >> --- >> sound/core/seq/seq_clientmgr.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c >> index 811b95b..4c93520 100644 >> --- a/sound/core/seq/seq_clientmgr.c >> +++ b/sound/core/seq/seq_clientmgr.c >> @@ -2122,7 +2122,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, >> * within 13 bits. We can safely pick up the size from the command. >> */ >> size = _IOC_SIZE(handler->cmd); >> - if (_IOC_DIR(handler->cmd) & IOC_IN) { >> + if (handler->cmd & IOC_IN) { >> if (copy_from_user(&buf, (const void __user *)arg, size)) >> return -EFAULT; >> } >> @@ -2132,7 +2132,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, >> /* Some commands includes a bug in 'dir' field. */ >> if (handler->cmd == SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT || >> handler->cmd == SNDRV_SEQ_IOCTL_SET_CLIENT_POOL || >> - (_IOC_DIR(handler->cmd) & IOC_OUT)) >> + (handler->cmd & IOC_OUT)) >> if (copy_to_user((void __user *)arg, &buf, size)) >> return -EFAULT; >> } >> -- >> 2.7.4
On Tue, 13 Sep 2016 19:46:45 +0200, Takashi Sakamoto wrote: > > On Sep 13 2016 22:47, Takashi Iwai wrote: > > On Tue, 13 Sep 2016 12:37:53 +0200, > > Takashi Sakamoto wrote: > >> > >> When checking value of request for copy operation, current implementation > >> compares shifted value to macros, while these macros are already shifted. > >> As a result, it never performs to copy from/to user space. > >> > >> This commit fixes the bug. > >> > >> Fixes: 8ce8eb601c71('ALSA: seq: add an alternative way to handle ioctl requests' > >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> > > > > Applied, thanks. > > Thanks to apply this, and sorry to have posted with such a critical bug... No problem, it's still the development tree. And it's good that you could catch it quickly. thanks, Takashi
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 811b95b..4c93520 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2122,7 +2122,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, * within 13 bits. We can safely pick up the size from the command. */ size = _IOC_SIZE(handler->cmd); - if (_IOC_DIR(handler->cmd) & IOC_IN) { + if (handler->cmd & IOC_IN) { if (copy_from_user(&buf, (const void __user *)arg, size)) return -EFAULT; } @@ -2132,7 +2132,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, /* Some commands includes a bug in 'dir' field. */ if (handler->cmd == SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT || handler->cmd == SNDRV_SEQ_IOCTL_SET_CLIENT_POOL || - (_IOC_DIR(handler->cmd) & IOC_OUT)) + (handler->cmd & IOC_OUT)) if (copy_to_user((void __user *)arg, &buf, size)) return -EFAULT; }
When checking value of request for copy operation, current implementation compares shifted value to macros, while these macros are already shifted. As a result, it never performs to copy from/to user space. This commit fixes the bug. Fixes: 8ce8eb601c71('ALSA: seq: add an alternative way to handle ioctl requests' Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/core/seq/seq_clientmgr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)