linux-kernel - Re: [PATCH] irq: move some interrupt arch_* functions into struct irq

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: <1268242912.11737.70762.camel@zakaz.uk.xensource.com>
Date:	Wed, 10 Mar 2010 17:41:52 +0000
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, Yinghai Lu <yinghai@...nel.org>,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	"x86@...nel.org" <x86@...nel.org>,
	"linuxppc-dev@...abs.org" <linuxppc-dev@...abs.org>
Subject: Re: [PATCH] irq: move some interrupt arch_* functions into struct
 irq_chip.

On Wed, 2010-03-10 at 17:18 +0000, Eric W. Biederman wrote:
> Ian Campbell <Ian.Campbell@...rix.com> writes:
> 
> > On Wed, 2010-03-10 at 10:55 +0000, ijc@...lion.org.uk wrote:
> >> 
> >> arch_init_chip_data cannot be moved into struct irq_chip at this time
> >> because irq_desc->chip is not known at the time the irq_desc is
> >> setup. For now rename arch_init_chip_data to arch_init_irq_desc (for
> >> PowerPC, the only other user, whose usage better matches the new name)
> >> and on x86 convert arch_init_chip_data to ioapic_init_chip_data and
> >> call this whenever the IO APIC code allocates a new IRQ.
> >
> > One idea I had to improve this was to add a struct irq_chip * as a
> > parameter to irq_to_desc_alloc_node. The new parameter potentially could
> > be NULL for current behaviour. Does that sound like a reasonable
> > approach?
> 
> I don't follow why we have the restriction that irq_to_desc_alloc_node
> must call arch_init_chip_data.  Assuming that requirement to call arch_init_chip_data
> is valid, passing something into init_one_irq_desc seems appropriate.

Yes, I suspect that could also be made to work.

The lifecycle of the irq_desc and chip_data isn't really clear to me --
I guess once allocated an irq_desc never gets freed (at least
currently)? The associated chip_data can be freed on migrate and
replaced with a new one, but is not freed otherwise.

My concern is that if the caller asks for an IRQ which already exists
(is that valid?) then you will get that existing irq_desc back,
including its existing chip_data, which potentially leaks the new one
which was passed in. Or is it the case that the only way this could
happen would be for legacy IRQs? In which case perhaps it is simply
invalid to pass a new chip data in for such an IRQ.

Ian.

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