Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751202AbWJEIHf (ORCPT ); Thu, 5 Oct 2006 04:07:35 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751193AbWJEIHf (ORCPT ); Thu, 5 Oct 2006 04:07:35 -0400 Received: from gate.perex.cz ([85.132.177.35]:13504 "EHLO gate.perex.cz") by vger.kernel.org with ESMTP id S1751202AbWJEIHe (ORCPT ); Thu, 5 Oct 2006 04:07:34 -0400 Date: Thu, 5 Oct 2006 10:07:31 +0200 (CEST) From: Jaroslav Kysela X-X-Sender: perex@tm8103.perex-int.cz To: Jiri Kosina Cc: Castet Matthieu , Takashi Iwai , LKML , ALSA development , Andrew Morton , Alan Stern , Greg Kroah-Hartman Subject: Re: [PATCH] ALSA: fix kernel panic in initialization of mpu401 driver In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2926 Lines: 73 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: [PATCH] drivers/base - check if device is registered before removal Without this fix platform_device_unregister() might oops. Signed-off-by: Jaroslav Kysela diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 12173d1..daa2390 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -428,8 +428,10 @@ void bus_remove_device(struct device * d sysfs_remove_link(&dev->kobj, "bus"); sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id); device_remove_attrs(dev->bus, dev); - dev->is_registered = 0; - klist_del(&dev->knode_bus); + if (dev->is_registered) { + dev->is_registered = 0; + klist_del(&dev->knode_bus); + } pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id); device_release_driver(dev); put_bus(dev->bus); Jaroslav ----- Jaroslav Kysela Linux Kernel Sound Maintainer ALSA Project, SUSE Labs - 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/