Hi,
On Wed, Oct 20, 2021 at 07:40:46AM +0200, Takashi Iwai wrote:
> On Wed, 20 Oct 2021 06:25:55 +0200,
> Takashi Sakamoto wrote:
> >
> > 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.
> >
> > Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model")
> > Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
>
> Wouldn't it be simpler to add the flag check in
> snd_motu_register_dsp_message_parser_count_event() and return 0 if
> SND_MOTU_SPEC_REGISTER_DSP isn't set there?
Indeed. It's simpler than my patch.
When posted, I considered about pre-condition in context of design by
contract. I programmed motu-register-dsp-message-parser.c so that
callers of extern functions should guarantee the target is register
DSP models.
> > +// NOTE: Take care of page fault due to accessing to userspace.
> > +static long copy_dsp_event_to_user(struct snd_motu *motu, char __user *buf, long count,
> > + struct snd_firewire_event_motu_register_dsp_change *event)
> > +{
> > + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) {
> > + size_t consumed = 0;
> > + u32 __user *ptr;
> > + u32 ev;
> > +
> > + // Header is filled later.
> > + consumed += sizeof(*event);
> > +
> > + while (consumed < count &&
> > + snd_motu_register_dsp_message_parser_copy_event(motu, &ev)) {
> > + ptr = (u32 __user *)(buf + consumed);
> > + if (put_user(ev, ptr))
> > + return -EFAULT;
> > + consumed += sizeof(ev);
> > + }
> > +
> > + event->type = SNDRV_FIREWIRE_EVENT_MOTU_REGISTER_DSP_CHANGE;
> > + event->count = (consumed - sizeof(*event)) / 4;
> > + if (copy_to_user(buf, &event, sizeof(*event)))
The second argument should have been 'event'...
Anyway I'll use a bit more time to consider about what is better to fix the
bug.
Thanks
Takashi Sakamoto
