linux-kernel - Re: [git patch] free

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]

Date:	Wed, 23 Apr 2008 22:10:35 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	Rene Herman <rene.herman@...access.nl>,
	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Adrian Bunk <bunk@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, rmk@....linux.org.uk,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>
Subject: Re: [git patch] free_irq() fixes

Rene Herman wrote:
> On 23-04-08 02:16, Linus Torvalds wrote:
> 
>> On Wed, 23 Apr 2008, Adrian Bunk wrote:
>>> If it goes like the regs removal in one big patch around -rc1 into 
>>> your tree this shouldn't be a problem.
>>
>> Well, the regs removal had a real upside (it wasn't even sensible for 
>> all irq types), and really nobody used it apart from "system users" 
>> (ie Sysrq etc).
>>
>> I'm still waiting for anybody mentioning any upside at _all_ on 
>> removing "irq".
> 
> Saves another 4 bytes of stack? :-/ Seriously, Jeff can probably better 
> answer himself but when this was posted before:
> 
> http://lkml.org/lkml/2007/5/19/23
> 
> Eric Biederman said it fit nicely into his "nefarious plan of making 
> everything use a struct irq pointer". A later mention:
> 
> http://lkml.org/lkml/2007/10/19/66
> 
> got strong ACKs from Thomas Gleixner, Ingo Molnar and Greg KH. Remember 
> due to working on a local driver at the time and deleting the "irq" 
> argument usage from its handler (unneccesarily used in a debugging 
> printk) from it in response.

Thanks.  I was hoping that some of the people who expressed interest in 
prior threads would appear.

Answering Linus's question, the things I tend to think of are

* it's not used in overwhelming majority of cases

* irq number has morphed over time with MSI-X and APICs and such from a 
direct "reference" to a hardware line to a more abstract cookie value.

* the need for a struct [pci_]device everywhere means drivers have ready 
access to irq number _anyway_

* it has clearly led to many helpful cleanups and bug fixes, by both me 
and others [and yes, for the sake of argument I'm excluding those 
discussed in this thread]

* it helps clean up abuses like HPET where it is used to encode data 
(ignoring dev_id unnecessarily...  I posted a patch to fix this):

         if (rtc_int_flag) {
                 rtc_int_flag |= (RTC_IRQF | (RTC_NUM_INTS << 8));
                 if (irq_handler)
                         irq_handler(rtc_int_flag, dev_id);
         }

["irq_handler" is a function passed to request_irq, as well as being 
called here]

dev_id exists for passing various data to the irq_handler...  with some 
drivers abusing the 'irq' argument to pass data, that potential opens 
holes for bugs whenever the irq numbering (aka cookie) scheme is changed 
-- because changing the cookie scheme could potentially trigger code like

	if (irq == MAGIC_NUMBER)
		this is an internal self-call, do some polling
	else
		handle real hardware-raised interrupt

When drivers make assumptions about system irq numbering, particularly 
on x86, IMO the situation is fragile.

	Jeff



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