From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CBE81C43219 for ; Tue, 30 Apr 2019 07:31:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A1C0A2080C for ; Tue, 30 Apr 2019 07:31:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726470AbfD3Hbf (ORCPT ); Tue, 30 Apr 2019 03:31:35 -0400 Received: from mx2.suse.de ([195.135.220.15]:47374 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725769AbfD3Hbe (ORCPT ); Tue, 30 Apr 2019 03:31:34 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 67FF2AE00; Tue, 30 Apr 2019 07:31:33 +0000 (UTC) Date: Tue, 30 Apr 2019 09:31:32 +0200 Message-ID: From: Takashi Iwai To: "Song liwei" Cc: , "Yu Zhao" , "Mark Brown" , "Keyon Jie" , "Jaroslav Kysela" , "linux-kernel" Subject: Re: [PATCH] ALSA: hda: check RIRB to avoid use NULL pointer In-Reply-To: <1556604653-47363-1-git-send-email-liwei.song@windriver.com> References: <1556604653-47363-1-git-send-email-liwei.song@windriver.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) 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 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 30 Apr 2019 08:10:53 +0200, Song liwei wrote: > > From: Liwei Song > > Fix the following BUG: > > BUG: unable to handle kernel NULL pointer dereference at 000000000000000c > Workqueue: events azx_probe_work [snd_hda_intel] > RIP: 0010:snd_hdac_bus_update_rirb+0x80/0x160 [snd_hda_core] > Call Trace: > > azx_interrupt+0x78/0x140 [snd_hda_codec] > __handle_irq_event_percpu+0x49/0x300 > handle_irq_event_percpu+0x23/0x60 > handle_irq_event+0x3c/0x60 > handle_edge_irq+0xdb/0x180 > handle_irq+0x23/0x30 > do_IRQ+0x6a/0x140 > common_interrupt+0xf/0xf > > The Call Trace happened when run kdump on a NFS rootfs system. > Exist the following calling sequence when boot the second kernel: > > azx_first_init() > --> azx_acquire_irq() > <-- interrupt come in, azx_interrupt() was called > --> hda_intel_init_chip() > --> azx_init_chip() > --> snd_hdac_bus_init_chip() > --> snd_hdac_bus_init_cmd_io(); > --> init rirb.buf and corb.buf > > Interrupt happened after azx_acquire_irq() while RIRB still didn't got > initialized, then NULL pointer will be used when process the interrupt. > > Check the value of RIRB to ensure it is not NULL, to aviod some special > case may hang the system. > > Fixes: 14752412721c ("ALSA: hda - Add the controller helper codes to hda-core module") > Signed-off-by: Liwei Song Oh, that's indeed a race there. But I guess the check introduced by the patch is still error-prone. Basically the interrupt handling should be moved after the chip initialization. I suppose that your platform uses the shared interrupt, not the MSI? In anyway, alternative (and likely more certain) fix would be to move the azx_acquir_irq() call like the patch below (note: totally untested). Could you check whether it works? thanks, Takashi --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1788,9 +1788,6 @@ static int azx_first_init(struct azx *chip) chip->msi = 0; } - if (azx_acquire_irq(chip, 0) < 0) - return -EBUSY; - pci_set_master(pci); synchronize_irq(bus->irq); @@ -1904,6 +1901,9 @@ static int azx_first_init(struct azx *chip) return -ENODEV; } + if (azx_acquire_irq(chip, 0) < 0) + return -EBUSY; + strcpy(card->driver, "HDA-Intel"); strlcpy(card->shortname, driver_short_names[chip->driver_type], sizeof(card->shortname));