linux-kernel - Re: [PATCH] Revert "[Bluetooth] Eliminate checks for impossible conditions in IRQ handler"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4AAECEF4.9070200@gmail.com>
Date:	Mon, 14 Sep 2009 17:17:08 -0600
From:	Robert Hancock <hancockrwd@...il.com>
To:	Mike Frysinger 
	<public-vapier-aBrp7R+bbdUdnm+yROfE0A@...ne.gmane.org>
CC:	Marcel Holtmann 
	<public-marcel-kz+m5ild9QBg9hUCZPvPmw@...ne.gmane.org>,
	public-linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@...ne.gmane.org,
	public-linux-kernel-u79uwXL29TY76Z2rM5mHXA@...ne.gmane.org,
	Michael Hennerich 
	<public-michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@...ne.gmane.org>
Subject: Re: [PATCH] Revert "[Bluetooth] Eliminate checks for impossible conditions
 in IRQ handler"



On 09/13/2009 11:18 PM, Mike Frysinger wrote:
> Commit ac019360fe3 changed the irq handler logic to BUG_ON rather than
> returning IRQ_NONE when the incoming argument is invalid.  While this
> works in most cases, it doesn't work when the IRQ is shared with other
> devices (or when DEBUG_SHIRQ is enabled).

Something doesn't add up here. It shouldn't be possible for the incoming 
argument to be invalid. I'd think that if it is it means that the IRQ 
handler is being registered too soon, before the data structures it 
requires are set up fully. If that's the case, reverting the change just 
partially papers over the bug.

>
> So revert the previous change and replace the warning message with a
> comment.
>
> Signed-off-by: Michael Hennerich<michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@...lic.gmane.org>
> Signed-off-by: Mike Frysinger<vapier-aBrp7R+bbdUdnm+yROfE0A@...lic.gmane.org>
> ---
>   drivers/bluetooth/bluecard_cs.c |    4 +++-
>   drivers/bluetooth/bt3c_cs.c     |    4 +++-
>   drivers/bluetooth/btuart_cs.c   |    4 +++-
>   drivers/bluetooth/dtl1_cs.c     |    4 +++-
>   4 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
> index b0e569b..a3a8f4d 100644
> --- a/drivers/bluetooth/bluecard_cs.c
> +++ b/drivers/bluetooth/bluecard_cs.c
> @@ -503,7 +503,9 @@ static irqreturn_t bluecard_interrupt(int irq, void *dev_inst)
>   	unsigned int iobase;
>   	unsigned char reg;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	if (!test_bit(CARD_READY,&(info->hw_state)))
>   		return IRQ_HANDLED;
> diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
> index d58e22b..9a7578e 100644
> --- a/drivers/bluetooth/bt3c_cs.c
> +++ b/drivers/bluetooth/bt3c_cs.c
> @@ -345,7 +345,9 @@ static irqreturn_t bt3c_interrupt(int irq, void *dev_inst)
>   	int iir;
>   	irqreturn_t r = IRQ_NONE;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	iobase = info->p_dev->io.BasePort1;
>
> diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
> index efd689a..cd9cb6c 100644
> --- a/drivers/bluetooth/btuart_cs.c
> +++ b/drivers/bluetooth/btuart_cs.c
> @@ -295,7 +295,9 @@ static irqreturn_t btuart_interrupt(int irq, void *dev_inst)
>   	int iir, lsr;
>   	irqreturn_t r = IRQ_NONE;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	iobase = info->p_dev->io.BasePort1;
>
> diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
> index 2cc7b32..588678a 100644
> --- a/drivers/bluetooth/dtl1_cs.c
> +++ b/drivers/bluetooth/dtl1_cs.c
> @@ -299,7 +299,9 @@ static irqreturn_t dtl1_interrupt(int irq, void *dev_inst)
>   	int iir, lsr;
>   	irqreturn_t r = IRQ_NONE;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	iobase = info->p_dev->io.BasePort1;
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/