linux-kernel - Re: [PATCH] Fix potential NULL pointer dereference in echoaudio midi.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5hu01csvb3.wl%tiwai@suse.de>
Date:	Mon, 06 Nov 2006 11:50:40 +0100
From:	Takashi Iwai <tiwai@...e.de>
To:	"Jesper Juhl" <jesper.juhl@...il.com>
Cc:	"Giuliano Pochini" <pochini@...ny.it>, rientjes@...washington.edu,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix potential NULL pointer dereference in echoaudio midi.

At Thu, 2 Nov 2006 21:22:43 +0100,
Jesper Juhl wrote:
> 
> On 02/11/06, Giuliano Pochini <pochini@...ny.it> wrote:
> > On Tue, 31 Oct 2006 23:26:31 +0100
> > Jesper Juhl <jesper.juhl@...il.com> wrote:
> >
> > > On Tuesday 31 October 2006 23:13, David Rientjes wrote:
> > > > On Tue, 31 Oct 2006, Jesper Juhl wrote:
> > > >
> > > > > In sound/pci/echoaudio/midi.c::snd_echo_midi_output_write(), there's a risk
> > > > > of dereferencing a NULL 'chip->midi_out'.
> > > > > This patch contains the obvious fix as also used a bit higher up in the
> > > > > same function.
> > > > >
> > > >
> > > > How about just adding an early test:
> > > >     if (!chip->midi_out)
> > > >             goto out;
> >
> >
> > The point of that check is to make sure is doesn't access chip->midi_out
> > when (surprise!) it is NULL. This can only happen in the rare (possible?)
> > case snd_echo_midi_output_close() is called while the timer handler is
> > running. I have another proposal which IMHO is smp-safer that just moving
> > the check. In that case we should also put a spinlock around the
> > chip->midi_out=0 in the snd_echo_midi_output_close() callback.
> >
> >
> > Signed-off-by: Giuliano Pochini <pochini@...ny.it>
> >
> > --- alsa-kernel/pci/echoaudio/midi.c__orig      2006-11-02 20:39:45.000000000 +0100
> > +++ alsa-kernel/pci/echoaudio/midi.c    2006-11-02 20:44:22.000000000 +0100
> > @@ -213,7 +213,7 @@ static void snd_echo_midi_output_write(u
> >         sent = bytes = 0;
> >         spin_lock_irqsave(&chip->lock, flags);
> >         chip->midi_full = 0;
> > -       if (chip->midi_out && !snd_rawmidi_transmit_empty(chip->midi_out)) {
> > +       if (!snd_rawmidi_transmit_empty(chip->midi_out)) {
> >                 bytes = snd_rawmidi_transmit_peek(chip->midi_out, buf,
> >                                                   MIDI_OUT_BUFFER_SIZE - 1);
> >                 DE_MID(("Try to send %d bytes...\n", bytes));
> > @@ -264,9 +264,11 @@ static void snd_echo_midi_output_trigger
> >                 }
> >         } else {
> >                 if (chip->tinuse) {
> > -                       del_timer(&chip->timer);
> >                         chip->tinuse = 0;
> > +                       spin_unlock_irq(&chip->lock);
> > +                       del_timer_sync(&chip->timer);
> >                         DE_MID(("Timer removed\n"));
> > +                       return;
> >                 }
> >         }
> >         spin_unlock_irq(&chip->lock);
> >
> 
> Fine by me.
> Let's just get one of the versions pushed into -mm or mainline :)

I applied it to ALSA tree mm branch.
Thanks.


Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/