linux-kernel - Re: [PATCH for -tip 4/4] irq: for_each_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 for Android: free password hash cracker in your pocket

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

Message-Id: <20090104030849.069A.KOSAKI.MOTOHIRO@jp.fujitsu.com>
Date:	Sun, 4 Jan 2009 03:11:05 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	"Raja R Harinath" <harinath@...rynot.org>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Yinghai Lu <yinghai@...nel.org>
Cc:	kosaki.motohiro@...fujitsu.com
Subject: Re: [PATCH for -tip 4/4] irq: for_each_irq_desc() makes simplify

> Hi
> 
> >>  # define for_each_irq_desc(irq, desc)                                        \
> >>       for (irq = 0, desc = irq_to_desc(irq); irq < nr_irqs;           \
> >> -          irq++, desc = irq_to_desc(irq))
> >> +          irq++, desc = irq_to_desc(irq))                            \
> >> +             if (desc)
> >> +
> >> +
> >>  # define for_each_irq_desc_reverse(irq, desc)                                \
> >>       for (irq = nr_irqs - 1, desc = irq_to_desc(irq); irq >= 0;      \
> >> -          irq--, desc = irq_to_desc(irq))
> >> +          irq--, desc = irq_to_desc(irq))                            \
> >> +             if (desc)
> >
> > I know this has gone in, but isn't this naked 'if' unsafe.  Consider the
> > following hypothetical code:
> >
> >  if (safe)
> >    for_each_irq_desc(irq, desc) {
> >       ...
> >    }
> >  else
> >    panic();
> >
> > With the macro definition above, the loop would panic() each time !desc,
> > and _not_ panic() when !safe.  I'd consider this behaviour to be
> > unexpected, to say the least :-)
> 
> Correct.
> 
> > The fix is to change the
> >
> >  if (desc)
> >
> > in the macro to
> >
> >  if (!desc) ; else
> 
> Ok. I'll do that.
> Very thanks for good reviewing.

Done.
Thnaks Raja.

Ingo, could you please apply this patch to -tip tree?


===
>From 6f1210e6912afd8ca07518e82f89ba92137302b2 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Date: Sun, 4 Jan 2009 02:37:26 +0900
Subject: [PATCH] sparseirq: fix for_each_irq_desc() nit

Raja reported for_each_irq_desc() has possibility unsafeness.
if anyone write folliwing code, for_each_irq_desc() doesn't works intetionally.
(Now, its code doesn't exist at all)

 if (safe)
   for_each_irq_desc(irq, desc) {
      ...
   }
 else
   panic();


Reported-by: Raja R Harinath <harinath@...rynot.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
CC: Ingo Molnar <mingo@...e.hu>
CC: Yinghai Lu <yinghai@...nel.org>
---
 include/linux/irqnr.h |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqnr.h b/include/linux/irqnr.h
index 5504a5c..99b91c6 100644
--- a/include/linux/irqnr.h
+++ b/include/linux/irqnr.h
@@ -23,13 +23,17 @@ extern struct irq_desc *irq_to_desc(unsigned int irq);
 # define for_each_irq_desc(irq, desc)					\
 	for (irq = 0, desc = irq_to_desc(irq); irq < nr_irqs;		\
 	     irq++, desc = irq_to_desc(irq))				\
-		if (desc)
+		if (!desc)						\
+			;						\
+		else
 
 
 # define for_each_irq_desc_reverse(irq, desc)				\
 	for (irq = nr_irqs - 1, desc = irq_to_desc(irq); irq >= 0;	\
 	     irq--, desc = irq_to_desc(irq))				\
-		if (desc)
+		if (!desc)						\
+			;						\
+		else
 
 #endif /* CONFIG_GENERIC_HARDIRQS */
 
-- 
1.5.3




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