linux-kernel - Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved

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] [day] [month] [year] [list]

Message-ID: <CAE9FiQUbWrAeavs-9Ze19FxWWn7dC1JA-69FfVhZSgwYCcohhA@mail.gmail.com>
Date:	Mon, 24 Feb 2014 17:31:43 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	Tony Luck <tony.luck@...el.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	Joerg Roedel <joro@...tes.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Sebastian Andrzej Siewior <sebastian@...akpoint.cc>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

On Sat, Feb 22, 2014 at 3:38 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
>
> OMG, you really mean that:
>
> +++ b/arch/alpha/kernel/irq_i8259.c
> @@ -92,6 +92,7 @@ init_i8259a_irqs(void)
>         outb(0xff, 0xA1);       /* mask all of 8259A-2 */
>
>         for (i = 0; i < 16; i++) {
> +               irq_alloc_desc_at(i, 0);
>                 irq_set_chip_and_handler(i, &i8259a_irq_type, handle_level_irq);
>         }
>
> You can't be serious about that. There are tons of ways to call into
> the core and access an irq descriptor aside of irq_set_chip* before it
> is potentially allocated.
>
> Are you going to analyze all of them and add an irq_alloc_desc_at()
> before that call?
>
> HELL, NO!
>
> I'm really tired of that.
>
> Stay away from kernel/irq/* and wait for people who are competent
> enough and willing to spend the extra thoughts to come up with
> solutions which are not completely ass backwards.

oh, no, sorry for annoying you again.

In the irq_alloc_desc_at() path for !SPARSE_IRQ, it should not touch anything
except setting that bit in allocated_irqs bitmap.

Or could just replace irq_reserve_irq in irq_set_chip(), in that way will avoid
touching arch codes.

like

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index dc04c16..02d6ad0 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -28,8 +28,13 @@
 int irq_set_chip(unsigned int irq, struct irq_chip *chip)
 {
     unsigned long flags;
-    struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
+    struct irq_desc *desc;
+
+#ifndef CONFIG_SPARSE_IRQ
+    irq_alloc_desc_at(irq, 0);
+#endif

+    desc = irq_get_desc_lock(irq, &flags, 0);
     if (!desc)
         return -EINVAL;

@@ -38,12 +43,7 @@ int irq_set_chip(unsigned int irq, struct irq_chip *chip)

     desc->irq_data.chip = chip;
     irq_put_desc_unlock(desc, flags);
-    /*
-     * For !CONFIG_SPARSE_IRQ make the irq show up in
-     * allocated_irqs. For the CONFIG_SPARSE_IRQ case, it is
-     * already marked, and this call is harmless.
-     */
-    irq_reserve_irq(irq);
+
     return 0;
 }
 EXPORT_SYMBOL(irq_set_chip);
--
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/