At Thu, 2 Nov 2006 21:22:43 +0100,
Jesper Juhl wrote:
>
> On 02/11/06, Giuliano Pochini <[email protected]> wrote:
> > On Tue, 31 Oct 2006 23:26:31 +0100
> > Jesper Juhl <[email protected]> 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 <[email protected]>
> >
> > --- 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
