Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750799AbWJESso (ORCPT ); Thu, 5 Oct 2006 14:48:44 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750830AbWJESsn (ORCPT ); Thu, 5 Oct 2006 14:48:43 -0400 Received: from mail.kroah.org ([69.55.234.183]:4033 "EHLO perch.kroah.org") by vger.kernel.org with ESMTP id S1750799AbWJESsm (ORCPT ); Thu, 5 Oct 2006 14:48:42 -0400 Date: Thu, 5 Oct 2006 10:58:52 -0700 From: Greg KH To: Jaroslav Kysela Cc: Alan Stern , Jiri Kosina , Castet Matthieu , Takashi Iwai , LKML , ALSA development , Andrew Morton Subject: Re: [PATCH] ALSA: fix kernel panic in initialization of mpu401 driver Message-ID: <20061005175852.GC15180@suse.de> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.13 (2006-08-11) Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2918 Lines: 63 On Thu, Oct 05, 2006 at 04:16:01PM +0200, Jaroslav Kysela wrote: > On Thu, 5 Oct 2006, Alan Stern wrote: > > > On Thu, 5 Oct 2006, Jaroslav Kysela wrote: > > > > > On Wed, 4 Oct 2006, Jiri Kosina wrote: > > > > > > > I am getting kernel panic (NULL pointer dereference) on boot, with kernel > > > > compiled with CONFIG_SND_MPU401_UART=y, on machine which does not have > > > > this piece of hardware. > > > > > > > > I have traced the problem down to > > > > sound/drivers/mpu401/mpu401.c:snd_mpu401_probe() returning EINVAL, when > > > > either port or IRQ parameters are not specified. > > > > > > > > In such case, the drivers/base/bus.c:bus_attach_device() does not perform > > > > klist_add_tail() call, but rather sets dev->is_registered to 0. This flag > > > > is however not checked by the driver, so later on, when > > > > alsa_card_mpu401_init() is called and platform_device_register_simple() > > > > fails, the following callchain happens, causing NULL pointer dereference: > > > > alsa_card_mpu401_init() -> platform_device_unregister() -> > > > > platform_device_del() -> device_del() -> bus_remove_device() -> > > > > klist_del() -> BOOM (the entry was not added to klist in > > > > bus_attach_device()). > > > > > > > > Proper solution is returning ENODEV from the ->probe() routine, which will > > > > be correctly handled then by the rest of the device-driver attaching > > > > subsystem (namely the retval check in bus_attach_device()). The following > > > > patch fixes the problem, please apply. > > > > > > Unfortunately, I do not think that it's a proper solution. I think that > > > platform device layer should play more nicely and if probe() fails for > > > a reason and if platform_device_register_simple() does not set > > > IS_ERR(), then platform_device_unregister() must be callable to free > > > all resources. > > > > > > Also, you've proposed to not fix all returns in snd_mpu401_probe() only > > > first two. > > > > > > I would reject this patch and fix drivers/base/bus.c. The problematic > > > change is in commit f2eaae197f4590c4d96f31b09b0ee9067421a95c and this > > > patch will probably fix it: > > > > Your patch is right as far as it goes, but it misses part of the problem. > > device_add() shouldn't ignore the return value from bus_attach_device(). > > That's why we ended up trying to unregister a device which never was > > properly registered in the first place. > > > > What do you think of this addition to your patch (untested)? > > Your addition looks good. Acked from me. Great, Alan, care to resend with a better description and the proper signed-off-by lines? thanks, greg k-h - 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/