From patchwork Fri Nov 16 08:39:34 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bugzilla-daemon@bugzilla.kernel.org X-Patchwork-Id: 199518 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 999DF2C0090 for ; Fri, 16 Nov 2012 19:39:41 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751075Ab2KPIjk (ORCPT ); Fri, 16 Nov 2012 03:39:40 -0500 Received: from mail.kernel.org ([198.145.19.201]:37843 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750937Ab2KPIjk (ORCPT ); Fri, 16 Nov 2012 03:39:40 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BCC38202A5 for ; Fri, 16 Nov 2012 08:39:38 +0000 (UTC) Received: from bugzilla.kernel.org (bugzilla.kernel.org [198.145.19.217]) by mail.kernel.org (Postfix) with ESMTP id 9B5E22029C for ; Fri, 16 Nov 2012 08:39:37 +0000 (UTC) Received: by bugzilla.kernel.org (Postfix, from userid 1000) id B4F9711FEE3; Fri, 16 Nov 2012 08:39:34 +0000 (UTC) From: bugzilla-daemon@bugzilla.kernel.org To: linux-ide@vger.kernel.org Subject: [Bug 49151] NULL pointer dereference in pata_acpi X-Bugzilla-Reason: None X-Bugzilla-Type: newchanged X-Bugzilla-Watch-Reason: AssignedTo io_ide@kernel-bugs.osdl.org X-Bugzilla-Product: IO/Storage X-Bugzilla-Component: IDE X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: aaron.lu@intel.com X-Bugzilla-Status: ASSIGNED X-Bugzilla-Priority: P1 X-Bugzilla-Assigned-To: io_ide@kernel-bugs.osdl.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: In-Reply-To: References: Auto-Submitted: auto-generated MIME-Version: 1.0 Message-Id: <20121116083934.B4F9711FEE3@bugzilla.kernel.org> Date: Fri, 16 Nov 2012 08:39:34 +0000 (UTC) X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org https://bugzilla.kernel.org/show_bug.cgi?id=49151 --- Comment #41 from Aaron Lu 2012-11-16 08:39:33 --- The problem I see is: During init time, identify command needs to be sent; pacpi_qc_issue is invoked, and if the chipset can not set timing independently for each drive, and the qc is not for the current drive, dma mode will be set for the target device if ata_dma_enabled returns true; At this time, ata_device->dma_mode is still un-initialized as 0, but ata_dma_enabled would treat this as valid, and the ata_timing table doesn't handle mode 0, so we get NULL. This problem only occurs when processing identify command. So I think we should init ata_device->dma_mode to 0xff when we are to reset the drive, and the real value will get set in ata_set_mode afterwards. This way, when pacpi_qc_issue is invoked, it will not attempt to set dma mode for the device. And for Szymon's bisect, I think the 'offending' commit actually fixed a problem of pata_acpi(maybe a long standing problem). And due to this fix, pata_acpi module triggered this bug. In previous kernels(pre the 'offending' commit Szymon bisected), the acpi_port_start function will always fail due to ap->acpi_handle is NULL, the reason is showed in the following call sequence: ata_host_start ap->ops->port_start -> pacpi_port_start -> where ap->acpi_handle is used ata_host_register ata_associate_acpi -> where the ap->acpi_handle is assigned So in previous kernels, pata_acpi module will always fail to init the controller, effectively hiding this bug. Please someone test the following patch, as I do not have a system to reproduce this: diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 3cc7096..e04cdc2 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2560,6 +2560,7 @@ int ata_bus_probe(struct ata_port *ap) * bus as we may be talking too fast. */ dev->pio_mode = XFER_PIO_0; + dev->dma_mode = 0xff; /* If the controller has a pio mode setup function * then use it to set the chipset to rights. Don't diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index e60437c..bf039b0 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2657,6 +2657,7 @@ int ata_eh_reset(struct ata_link *link, int classify, * bus as we may be talking too fast. */ dev->pio_mode = XFER_PIO_0; + dev->dma_mode = 0xff; /* If the controller has a pio mode setup function * then use it to set the chipset to rights. Don't