ALSA firewire-motu driver recently got support for event notification via ALSA HwDep interface for register DSP models. However, when polling ALSA HwDep cdev, the driver can cause null pointer dereference for the other models due to accessing to unallocated memory or uninitialized memory.

This commit fixes the bug by check the type of model before accessing to the memory.

Reported-by: kernel test robot lkp@intel.com Suggested-by: Takashi Iwai tiwai@suse.de Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model") Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/motu/motu-hwdep.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c index 9c2e457ce692..a900fc0e7644 100644 --- a/sound/firewire/motu/motu-hwdep.c +++ b/sound/firewire/motu/motu-hwdep.c @@ -16,6 +16,14 @@

#include "motu.h"

+static bool has_dsp_event(struct snd_motu *motu) +{ + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) + return (snd_motu_register_dsp_message_parser_count_event(motu) > 0); + else + return false; +} + static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, loff_t *offset) { @@ -25,8 +33,7 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count,

spin_lock_irq(&motu->lock);

- while (!motu->dev_lock_changed && motu->msg == 0 && - snd_motu_register_dsp_message_parser_count_event(motu) == 0) { + while (!motu->dev_lock_changed && motu->msg == 0 && !has_dsp_event(motu)) { prepare_to_wait(&motu->hwdep_wait, &wait, TASK_INTERRUPTIBLE); spin_unlock_irq(&motu->lock); schedule(); @@ -55,7 +62,7 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, count = min_t(long, count, sizeof(event)); if (copy_to_user(buf, &event, count)) return -EFAULT; - } else if (snd_motu_register_dsp_message_parser_count_event(motu) > 0) { + } else if (has_dsp_event(motu)) { size_t consumed = 0; u32 __user *ptr; u32 ev; @@ -94,8 +101,7 @@ static __poll_t hwdep_poll(struct snd_hwdep *hwdep, struct file *file, poll_wait(file, &motu->hwdep_wait, wait);

spin_lock_irq(&motu->lock); - if (motu->dev_lock_changed || motu->msg || - snd_motu_register_dsp_message_parser_count_event(motu) > 0) + if (motu->dev_lock_changed || motu->msg || has_dsp_event(motu)) events = EPOLLIN | EPOLLRDNORM; else events = 0;

In command DSP models, one meter information consists of 4 bytes for IEEE 764 floating point (binary32). In previous patch, it is exported to userspace as 32 bit storage since the storage is also handled in ALSA firewire-motu driver as well in kernel space in which floating point arithmetic is not preferable. On the other hand, ALSA firewire-motu driver doesn't perform floating point calculation. The driver just gather meter information from isochronous packets and fill structure fields for userspace.

In 'header' target of Kbuild, UAPI headers are processed before installed. In this timing, #ifdef macro with __KERNEL__ is removed. This mechanism is useful in the case so that the 32 bit storage can be accessible as u32 type in kernel space and float type in user space. We can see the same usage in ''struct acct_v3' in 'include/uapi/linux/acct.h'.

This commit is for the above idea. Additionally, due to message protocol, meter information is filled with 0xffffffff in the end of period but 0xffffffff is invalid as binary32. To avoid confusion in userspace application, the last two elements are left without any assignment.

Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- include/uapi/sound/firewire.h | 8 +++++--- sound/firewire/motu/motu-command-dsp-message-parser.c | 7 +++++-- 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 68eb0e43c052..39cf6eb75940 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -248,12 +248,14 @@ struct snd_firewire_motu_register_dsp_parameter { * * The structure expresses the part of DSP status for hardware meter. The 32 bit storage is * estimated to include IEEE 764 32 bit single precision floating point (binary32) value. It is - * expected to be linear value (not logarithm) for audio signal level between 0.0 and +1.0. However, - * the last two quadlets (data[398] and data[399]) are filled with 0xffffffff since they are the - * marker of one period. + * expected to be linear value (not logarithm) for audio signal level between 0.0 and +1.0. */ struct snd_firewire_motu_command_dsp_meter { +#ifdef __KERNEL__ __u32 data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; +#else + float data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; +#endif };

#endif /* _UAPI_SOUND_FIREWIRE_H_INCLUDED */ diff --git a/sound/firewire/motu/motu-command-dsp-message-parser.c b/sound/firewire/motu/motu-command-dsp-message-parser.c index 18689fcfb288..9efe4d364baf 100644 --- a/sound/firewire/motu/motu-command-dsp-message-parser.c +++ b/sound/firewire/motu/motu-command-dsp-message-parser.c @@ -141,12 +141,15 @@ void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const stru ++parser->fragment_pos;

if (parser->fragment_pos == 4) { + // Skip the last two quadlets since they could be + // invalid value (0xffffffff) as floating point + // number. if (parser->value_index < - SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT) { + SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT - 2) { u32 val = (u32)(parser->value >> 32); parser->meter.data[parser->value_index] = val; - ++parser->value_index; } + ++parser->value_index; parser->fragment_pos = 0; }