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=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 371B7C433B4 for ; Sun, 16 May 2021 09:49:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 064E961168 for ; Sun, 16 May 2021 09:49:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230330AbhEPJum (ORCPT ); Sun, 16 May 2021 05:50:42 -0400 Received: from mx2.suse.de ([195.135.220.15]:40478 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229568AbhEPJul (ORCPT ); Sun, 16 May 2021 05:50:41 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 2E5E4B04F; Sun, 16 May 2021 09:49:26 +0000 (UTC) Date: Sun, 16 May 2021 11:49:26 +0200 Message-ID: From: Takashi Iwai To: Sergey Senozhatsky Cc: Jaroslav Kysela , Takashi Iwai , "Gustavo A. R. Silva" , Leon Romanovsky , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: ALSA: intel8x0: div by zero in snd_intel8x0_update() In-Reply-To: References: 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 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 16 May 2021 10:31:41 +0200, Sergey Senozhatsky wrote: > > On (21/05/16 17:30), Sergey Senozhatsky wrote: > > On (21/05/14 20:16), Sergey Senozhatsky wrote: > > > > --- a/sound/pci/intel8x0.c > > > > +++ b/sound/pci/intel8x0.c > > > > @@ -691,6 +691,9 @@ static inline void snd_intel8x0_update(struct intel8x0 *chip, struct ichdev *ich > > > > int status, civ, i, step; > > > > int ack = 0; > > > > > > > > + if (!ichdev->substream || ichdev->suspended) > > > > + return; > > > > + > > > > spin_lock_irqsave(&chip->reg_lock, flags); > > > > status = igetbyte(chip, port + ichdev->roff_sr); > > > > civ = igetbyte(chip, port + ICH_REG_OFF_CIV); > > > > This does the problem for me. > > ^^^ does fix OK, thanks for confirmation. So this looks like some spurious interrupt with the unexpected hardware bits. However, the suggested check doesn't seem covering enough, and it might still hit if the suspend/resume happens before the device is opened but not set up (and such a spurious irq is triggered). Below is more comprehensive fix. Let me know if this works, too. thanks, Takashi -- 8< -- Subject: [PATCH] ALSA: intel8x0: Don't update period unless prepared The interrupt handler of intel8x0 calls snd_intel8x0_update() whenever the hardware sets the corresponding status bit for each stream. This works fine for most cases as long as the hardware behaves properly. But when the hardware gives a wrong bit set, this leads to a NULL dereference Oops, and reportedly, this seems what happened on a VM. For fixing the crash, this patch adds a internal flag indicating that the stream is ready to be updated, and check it (as well as the flag being in suspended) to ignore such spurious update. Cc: Reported-by: Sergey Senozhatsky Signed-off-by: Takashi Iwai --- sound/pci/intel8x0.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 35903d1a1cbd..5b124c4ad572 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -331,6 +331,7 @@ struct ichdev { unsigned int ali_slot; /* ALI DMA slot */ struct ac97_pcm *pcm; int pcm_open_flag; + unsigned int prepared:1; unsigned int suspended: 1; }; @@ -691,6 +692,9 @@ static inline void snd_intel8x0_update(struct intel8x0 *chip, struct ichdev *ich int status, civ, i, step; int ack = 0; + if (!ichdev->prepared || ichdev->suspended) + return; + spin_lock_irqsave(&chip->reg_lock, flags); status = igetbyte(chip, port + ichdev->roff_sr); civ = igetbyte(chip, port + ICH_REG_OFF_CIV); @@ -881,6 +885,7 @@ static int snd_intel8x0_hw_params(struct snd_pcm_substream *substream, if (ichdev->pcm_open_flag) { snd_ac97_pcm_close(ichdev->pcm); ichdev->pcm_open_flag = 0; + ichdev->prepared = 0; } err = snd_ac97_pcm_open(ichdev->pcm, params_rate(hw_params), params_channels(hw_params), @@ -902,6 +907,7 @@ static int snd_intel8x0_hw_free(struct snd_pcm_substream *substream) if (ichdev->pcm_open_flag) { snd_ac97_pcm_close(ichdev->pcm); ichdev->pcm_open_flag = 0; + ichdev->prepared = 0; } return 0; } @@ -976,6 +982,7 @@ static int snd_intel8x0_pcm_prepare(struct snd_pcm_substream *substream) ichdev->pos_shift = (runtime->sample_bits > 16) ? 2 : 1; } snd_intel8x0_setup_periods(chip, ichdev); + ichdev->prepared = 1; return 0; } -- 2.26.2 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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 6D2BBC43460 for ; Sun, 16 May 2021 09:50:34 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F3C8461176 for ; Sun, 16 May 2021 09:50:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F3C8461176 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id B7980169C; Sun, 16 May 2021 11:49:40 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz B7980169C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1621158630; bh=SXNNDVE6iWWGdzgam18jU8uvoJX1gzebIr5zTLo0ryY=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=ITk851/ybNG0xZJQcFnPyuD5bpM4CVV4zELXlZFQVEI4jLxv6/LX9266u8JOje0MU K6JzDEc+TTCbjrNdtQnMuG4Oxps/07GhURUrKv5G2egcYTbIMhlzXZaMKklyBJE1T2 JjgpIHS0xgWL2whLF7O5htWXG7LpWguqt2TNx34I= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 4C4E5F801DB; Sun, 16 May 2021 11:49:40 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 4091CF80217; Sun, 16 May 2021 11:49:38 +0200 (CEST) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 1A474F8016E for ; Sun, 16 May 2021 11:49:31 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 1A474F8016E X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 2E5E4B04F; Sun, 16 May 2021 09:49:26 +0000 (UTC) Date: Sun, 16 May 2021 11:49:26 +0200 Message-ID: From: Takashi Iwai To: Sergey Senozhatsky Subject: Re: ALSA: intel8x0: div by zero in snd_intel8x0_update() In-Reply-To: References: 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 Cc: alsa-devel@alsa-project.org, Leon Romanovsky , Takashi Iwai , linux-kernel@vger.kernel.org, "Gustavo A. R. Silva" X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Sun, 16 May 2021 10:31:41 +0200, Sergey Senozhatsky wrote: > > On (21/05/16 17:30), Sergey Senozhatsky wrote: > > On (21/05/14 20:16), Sergey Senozhatsky wrote: > > > > --- a/sound/pci/intel8x0.c > > > > +++ b/sound/pci/intel8x0.c > > > > @@ -691,6 +691,9 @@ static inline void snd_intel8x0_update(struct intel8x0 *chip, struct ichdev *ich > > > > int status, civ, i, step; > > > > int ack = 0; > > > > > > > > + if (!ichdev->substream || ichdev->suspended) > > > > + return; > > > > + > > > > spin_lock_irqsave(&chip->reg_lock, flags); > > > > status = igetbyte(chip, port + ichdev->roff_sr); > > > > civ = igetbyte(chip, port + ICH_REG_OFF_CIV); > > > > This does the problem for me. > > ^^^ does fix OK, thanks for confirmation. So this looks like some spurious interrupt with the unexpected hardware bits. However, the suggested check doesn't seem covering enough, and it might still hit if the suspend/resume happens before the device is opened but not set up (and such a spurious irq is triggered). Below is more comprehensive fix. Let me know if this works, too. thanks, Takashi -- 8< -- Subject: [PATCH] ALSA: intel8x0: Don't update period unless prepared The interrupt handler of intel8x0 calls snd_intel8x0_update() whenever the hardware sets the corresponding status bit for each stream. This works fine for most cases as long as the hardware behaves properly. But when the hardware gives a wrong bit set, this leads to a NULL dereference Oops, and reportedly, this seems what happened on a VM. For fixing the crash, this patch adds a internal flag indicating that the stream is ready to be updated, and check it (as well as the flag being in suspended) to ignore such spurious update. Cc: Reported-by: Sergey Senozhatsky Signed-off-by: Takashi Iwai --- sound/pci/intel8x0.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 35903d1a1cbd..5b124c4ad572 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -331,6 +331,7 @@ struct ichdev { unsigned int ali_slot; /* ALI DMA slot */ struct ac97_pcm *pcm; int pcm_open_flag; + unsigned int prepared:1; unsigned int suspended: 1; }; @@ -691,6 +692,9 @@ static inline void snd_intel8x0_update(struct intel8x0 *chip, struct ichdev *ich int status, civ, i, step; int ack = 0; + if (!ichdev->prepared || ichdev->suspended) + return; + spin_lock_irqsave(&chip->reg_lock, flags); status = igetbyte(chip, port + ichdev->roff_sr); civ = igetbyte(chip, port + ICH_REG_OFF_CIV); @@ -881,6 +885,7 @@ static int snd_intel8x0_hw_params(struct snd_pcm_substream *substream, if (ichdev->pcm_open_flag) { snd_ac97_pcm_close(ichdev->pcm); ichdev->pcm_open_flag = 0; + ichdev->prepared = 0; } err = snd_ac97_pcm_open(ichdev->pcm, params_rate(hw_params), params_channels(hw_params), @@ -902,6 +907,7 @@ static int snd_intel8x0_hw_free(struct snd_pcm_substream *substream) if (ichdev->pcm_open_flag) { snd_ac97_pcm_close(ichdev->pcm); ichdev->pcm_open_flag = 0; + ichdev->prepared = 0; } return 0; } @@ -976,6 +982,7 @@ static int snd_intel8x0_pcm_prepare(struct snd_pcm_substream *substream) ichdev->pos_shift = (runtime->sample_bits > 16) ? 2 : 1; } snd_intel8x0_setup_periods(chip, ichdev); + ichdev->prepared = 1; return 0; } -- 2.26.2