Messages in this thread | | | Date | Thu, 2 Nov 2006 21:22:43 +0100 | From | "Jesper Juhl" <> | Subject | Re: [PATCH] Fix potential NULL pointer dereference in echoaudio midi. |
| |
On 02/11/06, Giuliano Pochini <pochini@shiny.it> wrote: > On Tue, 31 Oct 2006 23:26:31 +0100 > Jesper Juhl <jesper.juhl@gmail.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@shiny.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 :)
-- Jesper Juhl <jesper.juhl@gmail.com> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |