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>] [day] [month] [year] [list]
Message-Id: <37995C30-BB76-417A-B499-DC0DA6418139@kernel.crashing.org>
Date:	Wed, 25 Oct 2006 00:41:31 -0500
From:	Kumar Gala <galak@...nel.crashing.org>
To:	tglx@...utronix.de, Ingo Molnar <mingo@...e.hu>
Cc:	linuxppc-dev list <linuxppc-dev@...abs.org>,
	Scott Wood <scottwood@...escale.com>,
	"linux-kernel@...r.kernel.org mailing list" 
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] IPIC: Don't call set_irq_handler with desc->lock held.


On Oct 23, 2006, at 11:35 AM, Scott Wood wrote:

> This patch causes ipic_set_irq_type to set the handler directly rather
> than call set_irq_handler, which causes spinlock recursion because
> the lock is already held when ipic_set_irq_type is called.
>
> I'm also not convinced that ipic_set_irq_type should be changing the
> handler at all.  There seem to be several controllers that don't and
> several that do.  Those that do would break what appears to be a  
> common
> usage of calling set_irq_chip_and_handler followed by set_irq_type,  
> if a
> non-standard handler were to be used.  OTOH, irq_create_of_mapping()
> doesn't set the handler, but only calls set_irq_type().
>
> This patch gets things working in the spinlock-debugging-enabled case,
> but I'm curious as to where the handler setting is ideally supposed  
> to be
> done.  I don't see any documentation on set_irq_type() that clarifies
> what the semantics are supposed to be.

Guys, Scott pointed this problem out on a PPC interrupt controller,  
and wanted to raise it in a larger forum since it appears to exist on  
at least one ARM interrupt controller I looked at (ixp4xx).  What is  
the proper solution to handle this.

The callers of set_type() I found all grab desc->lock.

- kumar

>
> Signed-off-by: Scott Wood <scottwood@...escale.com>
> ---
>  arch/powerpc/sysdev/ipic.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
> index bc4d4a7..746f78c 100644
> --- a/arch/powerpc/sysdev/ipic.c
> +++ b/arch/powerpc/sysdev/ipic.c
> @@ -473,9 +473,9 @@ static int ipic_set_irq_type(unsigned in
>  	desc->status |= flow_type & IRQ_TYPE_SENSE_MASK;
>  	if (flow_type & IRQ_TYPE_LEVEL_LOW)  {
>  		desc->status |= IRQ_LEVEL;
> -		set_irq_handler(virq, handle_level_irq);
> +		desc->handle_irq = handle_level_irq;
>  	} else {
> -		set_irq_handler(virq, handle_edge_irq);
> +		desc->handle_irq = handle_edge_irq;
>  	}
>
>  	/* only EXT IRQ senses are programmable on ipic
> -- 
> 1.4.2.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@...abs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

-
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ