Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751010AbWKFKus (ORCPT ); Mon, 6 Nov 2006 05:50:48 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751191AbWKFKus (ORCPT ); Mon, 6 Nov 2006 05:50:48 -0500 Received: from ns1.suse.de ([195.135.220.2]:51382 "EHLO mx1.suse.de") by vger.kernel.org with ESMTP id S1751010AbWKFKur (ORCPT ); Mon, 6 Nov 2006 05:50:47 -0500 Date: Mon, 06 Nov 2006 11:50:40 +0100 Message-ID: From: Takashi Iwai To: "Jesper Juhl" Cc: "Giuliano Pochini" , rientjes@cs.washington.edu, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Fix potential NULL pointer dereference in echoaudio midi. In-Reply-To: <9a8748490611021222q7ca5aec1v2940185ffb7abbb2@mail.gmail.com> References: <200610312221.41089.jesper.juhl@gmail.com> <200610312326.31526.jesper.juhl@gmail.com> <20061102211609.7263ef9c.pochini@shiny.it> <9a8748490611021222q7ca5aec1v2940185ffb7abbb2@mail.gmail.com> User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 MULE XEmacs/21.5 (beta25) (eggplant) (+CVS-20060326) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2850 Lines: 70 At Thu, 2 Nov 2006 21:22:43 +0100, Jesper Juhl wrote: > > On 02/11/06, Giuliano Pochini wrote: > > On Tue, 31 Oct 2006 23:26:31 +0100 > > Jesper Juhl 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 > > > > --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/