* [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq @ 2011-11-10 10:53 Jamie Iles 2011-11-10 11:14 ` Jamie Iles 2011-11-16 11:34 ` Jamie Iles 0 siblings, 2 replies; 8+ messages in thread From: Jamie Iles @ 2011-11-10 10:53 UTC (permalink / raw) To: linux-kernel; +Cc: Jamie Iles, Thomas Gleixner, Grant Likely It is optional for an irqdomain to have a to_irq() method, and for simple domains they often don't require any operations at all - just hwirq to Linux irq translation. Check we have valid ops before dereferencing them. Patch originally by Rob Herring. Suggested-by: Rob Herring <robherring2@gmail.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Grant Likely <grant.likely@secretlab.ca> Signed-off-by: Jamie Iles <jamie@jamieiles.com> --- Rob, I can't see that you've already posted this but I didn't want to hold Marc's GIC patches up. include/linux/irqdomain.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index 99834e58..78a1e66 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -74,7 +74,7 @@ struct irq_domain { static inline unsigned int irq_domain_to_irq(struct irq_domain *d, unsigned long hwirq) { - if (d->ops->to_irq) + if (d->ops && d->ops->to_irq) return d->ops->to_irq(d, hwirq); if (WARN_ON(hwirq < d->hwirq_base)) return 0; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-10 10:53 [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq Jamie Iles @ 2011-11-10 11:14 ` Jamie Iles 2011-11-10 13:37 ` Rob Herring 2011-11-16 11:34 ` Jamie Iles 1 sibling, 1 reply; 8+ messages in thread From: Jamie Iles @ 2011-11-10 11:14 UTC (permalink / raw) To: Jamie Iles; +Cc: linux-kernel, Thomas Gleixner, Grant Likely, Rob Herring On Thu, Nov 10, 2011 at 10:53:03AM +0000, Jamie Iles wrote: > It is optional for an irqdomain to have a to_irq() method, and for > simple domains they often don't require any operations at all - just > hwirq to Linux irq translation. Check we have valid ops before > dereferencing them. > > Patch originally by Rob Herring. > > Suggested-by: Rob Herring <robherring2@gmail.com> Naturally git send-email doesn't know how to convert this into a CC, so Rob is now CC'd! Jamie > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Grant Likely <grant.likely@secretlab.ca> > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > --- > > Rob, I can't see that you've already posted this but I didn't want to > hold Marc's GIC patches up. > > include/linux/irqdomain.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > index 99834e58..78a1e66 100644 > --- a/include/linux/irqdomain.h > +++ b/include/linux/irqdomain.h > @@ -74,7 +74,7 @@ struct irq_domain { > static inline unsigned int irq_domain_to_irq(struct irq_domain *d, > unsigned long hwirq) > { > - if (d->ops->to_irq) > + if (d->ops && d->ops->to_irq) > return d->ops->to_irq(d, hwirq); > if (WARN_ON(hwirq < d->hwirq_base)) > return 0; > -- > 1.7.4.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-10 11:14 ` Jamie Iles @ 2011-11-10 13:37 ` Rob Herring 2011-11-23 12:23 ` Jamie Iles 0 siblings, 1 reply; 8+ messages in thread From: Rob Herring @ 2011-11-10 13:37 UTC (permalink / raw) To: Jamie Iles; +Cc: linux-kernel, Thomas Gleixner, Grant Likely On 11/10/2011 05:14 AM, Jamie Iles wrote: > On Thu, Nov 10, 2011 at 10:53:03AM +0000, Jamie Iles wrote: >> It is optional for an irqdomain to have a to_irq() method, and for >> simple domains they often don't require any operations at all - just >> hwirq to Linux irq translation. Check we have valid ops before >> dereferencing them. >> >> Patch originally by Rob Herring. >> >> Suggested-by: Rob Herring <robherring2@gmail.com> > > Naturally git send-email doesn't know how to convert this into a CC, so > Rob is now CC'd! > > Jamie > >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Grant Likely <grant.likely@secretlab.ca> >> Signed-off-by: Jamie Iles <jamie@jamieiles.com> >> --- >> >> Rob, I can't see that you've already posted this but I didn't want to >> hold Marc's GIC patches up. Thanks. I didn't get around to it yesterday. Acked-by: Rob Herring <rob.herring@calxeda.com> Rob >> >> include/linux/irqdomain.h | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h >> index 99834e58..78a1e66 100644 >> --- a/include/linux/irqdomain.h >> +++ b/include/linux/irqdomain.h >> @@ -74,7 +74,7 @@ struct irq_domain { >> static inline unsigned int irq_domain_to_irq(struct irq_domain *d, >> unsigned long hwirq) >> { >> - if (d->ops->to_irq) >> + if (d->ops && d->ops->to_irq) >> return d->ops->to_irq(d, hwirq); >> if (WARN_ON(hwirq < d->hwirq_base)) >> return 0; >> -- >> 1.7.4.1 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-10 13:37 ` Rob Herring @ 2011-11-23 12:23 ` Jamie Iles 2011-11-25 15:35 ` Jamie Iles 0 siblings, 1 reply; 8+ messages in thread From: Jamie Iles @ 2011-11-23 12:23 UTC (permalink / raw) To: Rob Herring Cc: Jamie Iles, linux-kernel, Thomas Gleixner, Grant Likely, Mark Brown Hi Rob, On Thu, Nov 10, 2011 at 07:37:32AM -0600, Rob Herring wrote: > On 11/10/2011 05:14 AM, Jamie Iles wrote: > > On Thu, Nov 10, 2011 at 10:53:03AM +0000, Jamie Iles wrote: > >> It is optional for an irqdomain to have a to_irq() method, and for > >> simple domains they often don't require any operations at all - just > >> hwirq to Linux irq translation. Check we have valid ops before > >> dereferencing them. > >> > >> Patch originally by Rob Herring. > >> > >> Suggested-by: Rob Herring <robherring2@gmail.com> > > > > Naturally git send-email doesn't know how to convert this into a CC, so > > Rob is now CC'd! > > > > Jamie > > > >> Cc: Thomas Gleixner <tglx@linutronix.de> > >> Cc: Grant Likely <grant.likely@secretlab.ca> > >> Signed-off-by: Jamie Iles <jamie@jamieiles.com> > >> --- > >> > >> Rob, I can't see that you've already posted this but I didn't want to > >> hold Marc's GIC patches up. > > Thanks. I didn't get around to it yesterday. > > Acked-by: Rob Herring <rob.herring@calxeda.com> I've just noticed in kernel/irq/irqdomain.c, that the kerneldoc for irq_domain_add() says that a valid ops structure is required, so perhaps this fix isn't right and we should just have an empty ops structure for simple controllers... Jamie ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-23 12:23 ` Jamie Iles @ 2011-11-25 15:35 ` Jamie Iles 2011-11-27 15:11 ` Rob Herring 0 siblings, 1 reply; 8+ messages in thread From: Jamie Iles @ 2011-11-25 15:35 UTC (permalink / raw) To: Jamie Iles, Rob Herring Cc: Rob Herring, linux-kernel, Thomas Gleixner, Grant Likely, Mark Brown Hi Rob, On Wed, Nov 23, 2011 at 12:23:08PM +0000, Jamie Iles wrote: > On Thu, Nov 10, 2011 at 07:37:32AM -0600, Rob Herring wrote: > > On 11/10/2011 05:14 AM, Jamie Iles wrote: > > > On Thu, Nov 10, 2011 at 10:53:03AM +0000, Jamie Iles wrote: > > >> It is optional for an irqdomain to have a to_irq() method, and for > > >> simple domains they often don't require any operations at all - just > > >> hwirq to Linux irq translation. Check we have valid ops before > > >> dereferencing them. > > >> > > >> Patch originally by Rob Herring. > > >> > > >> Suggested-by: Rob Herring <robherring2@gmail.com> > > > > > > Naturally git send-email doesn't know how to convert this into a CC, so > > > Rob is now CC'd! > > > > > > Jamie > > > > > >> Cc: Thomas Gleixner <tglx@linutronix.de> > > >> Cc: Grant Likely <grant.likely@secretlab.ca> > > >> Signed-off-by: Jamie Iles <jamie@jamieiles.com> > > >> --- > > >> > > >> Rob, I can't see that you've already posted this but I didn't want to > > >> hold Marc's GIC patches up. > > > > Thanks. I didn't get around to it yesterday. > > > > Acked-by: Rob Herring <rob.herring@calxeda.com> > > I've just noticed in kernel/irq/irqdomain.c, that the kerneldoc for > irq_domain_add() says that a valid ops structure is required, so perhaps > this fix isn't right and we should just have an empty ops structure for > simple controllers... I wonder if this might make more sense instead. I'll post a proper patch if you think so. Regards, Jamie diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 200ce83..92c3484 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -143,11 +143,6 @@ int irq_domain_simple_dt_translate(struct irq_domain *d, return 0; } -struct irq_domain_ops irq_domain_simple_ops = { - .dt_translate = irq_domain_simple_dt_translate, -}; -EXPORT_SYMBOL_GPL(irq_domain_simple_ops); - /** * irq_domain_create_simple() - Set up a 'simple' translation range */ @@ -181,4 +176,11 @@ void irq_domain_generate_simple(const struct of_device_id *match, pr_info("no node found\n"); } EXPORT_SYMBOL_GPL(irq_domain_generate_simple); +#else /* CONFIG_OF_IRQ */ +#define irq_domain_simple_dt_translate NULL #endif /* CONFIG_OF_IRQ */ + +struct irq_domain_ops irq_domain_simple_ops = { + .dt_translate = irq_domain_simple_dt_translate, +}; +EXPORT_SYMBOL_GPL(irq_domain_simple_ops); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-25 15:35 ` Jamie Iles @ 2011-11-27 15:11 ` Rob Herring 2011-11-27 15:24 ` Jamie Iles 0 siblings, 1 reply; 8+ messages in thread From: Rob Herring @ 2011-11-27 15:11 UTC (permalink / raw) To: Jamie Iles Cc: linux-kernel, Thomas Gleixner, Grant Likely, Mark Brown, devicetree-discuss Jamie, On 11/25/2011 09:35 AM, Jamie Iles wrote: > Hi Rob, > > On Wed, Nov 23, 2011 at 12:23:08PM +0000, Jamie Iles wrote: >> On Thu, Nov 10, 2011 at 07:37:32AM -0600, Rob Herring wrote: >>> On 11/10/2011 05:14 AM, Jamie Iles wrote: >>>> On Thu, Nov 10, 2011 at 10:53:03AM +0000, Jamie Iles wrote: >>>>> It is optional for an irqdomain to have a to_irq() method, and for >>>>> simple domains they often don't require any operations at all - just >>>>> hwirq to Linux irq translation. Check we have valid ops before >>>>> dereferencing them. >>>>> >>>>> Patch originally by Rob Herring. >>>>> >>>>> Suggested-by: Rob Herring <robherring2@gmail.com> >>>> >>>> Naturally git send-email doesn't know how to convert this into a CC, so >>>> Rob is now CC'd! >>>> >>>> Jamie >>>> >>>>> Cc: Thomas Gleixner <tglx@linutronix.de> >>>>> Cc: Grant Likely <grant.likely@secretlab.ca> >>>>> Signed-off-by: Jamie Iles <jamie@jamieiles.com> >>>>> --- >>>>> >>>>> Rob, I can't see that you've already posted this but I didn't want to >>>>> hold Marc's GIC patches up. >>> >>> Thanks. I didn't get around to it yesterday. >>> >>> Acked-by: Rob Herring <rob.herring@calxeda.com> >> >> I've just noticed in kernel/irq/irqdomain.c, that the kerneldoc for >> irq_domain_add() says that a valid ops structure is required, so perhaps >> this fix isn't right and we should just have an empty ops structure for >> simple controllers... > > I wonder if this might make more sense instead. I'll post a proper > patch if you think so. > > Regards, > > Jamie > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index 200ce83..92c3484 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -143,11 +143,6 @@ int irq_domain_simple_dt_translate(struct irq_domain *d, > return 0; > } > > -struct irq_domain_ops irq_domain_simple_ops = { > - .dt_translate = irq_domain_simple_dt_translate, > -}; > -EXPORT_SYMBOL_GPL(irq_domain_simple_ops); > - > /** > * irq_domain_create_simple() - Set up a 'simple' translation range > */ > @@ -181,4 +176,11 @@ void irq_domain_generate_simple(const struct of_device_id *match, > pr_info("no node found\n"); > } > EXPORT_SYMBOL_GPL(irq_domain_generate_simple); > +#else /* CONFIG_OF_IRQ */ > +#define irq_domain_simple_dt_translate NULL > #endif /* CONFIG_OF_IRQ */ > + > +struct irq_domain_ops irq_domain_simple_ops = { > + .dt_translate = irq_domain_simple_dt_translate, .dt_translate is surrounded by CONFIG_OF, so it will break on !CONFIG_OF. > +}; > +EXPORT_SYMBOL_GPL(irq_domain_simple_ops); The extern declaration in irqdomain.h is surrounded by CONFIG_OF_IRQ, so it needs to be moved out of that as well. On Sparc, CONFIG_OF is true, but CONFIG_OF_IRQ is false. So that combination needs to be handled as well. Rob ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-27 15:11 ` Rob Herring @ 2011-11-27 15:24 ` Jamie Iles 0 siblings, 0 replies; 8+ messages in thread From: Jamie Iles @ 2011-11-27 15:24 UTC (permalink / raw) To: Rob Herring Cc: Jamie Iles, linux-kernel, Thomas Gleixner, Grant Likely, Mark Brown, devicetree-discuss Hi Rob, On Sun, Nov 27, 2011 at 09:11:01AM -0600, Rob Herring wrote: > On 11/25/2011 09:35 AM, Jamie Iles wrote: > > I wonder if this might make more sense instead. I'll post a proper > > patch if you think so. > > > > Regards, > > > > Jamie > > > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > > index 200ce83..92c3484 100644 > > --- a/kernel/irq/irqdomain.c > > +++ b/kernel/irq/irqdomain.c > > @@ -143,11 +143,6 @@ int irq_domain_simple_dt_translate(struct irq_domain *d, > > return 0; > > } > > > > -struct irq_domain_ops irq_domain_simple_ops = { > > - .dt_translate = irq_domain_simple_dt_translate, > > -}; > > -EXPORT_SYMBOL_GPL(irq_domain_simple_ops); > > - > > /** > > * irq_domain_create_simple() - Set up a 'simple' translation range > > */ > > @@ -181,4 +176,11 @@ void irq_domain_generate_simple(const struct of_device_id *match, > > pr_info("no node found\n"); > > } > > EXPORT_SYMBOL_GPL(irq_domain_generate_simple); > > +#else /* CONFIG_OF_IRQ */ > > +#define irq_domain_simple_dt_translate NULL > > #endif /* CONFIG_OF_IRQ */ > > + > > +struct irq_domain_ops irq_domain_simple_ops = { > > + .dt_translate = irq_domain_simple_dt_translate, > > .dt_translate is surrounded by CONFIG_OF, so it will break on !CONFIG_OF. Ahh, good spot! > > +}; > > +EXPORT_SYMBOL_GPL(irq_domain_simple_ops); > > The extern declaration in irqdomain.h is surrounded by CONFIG_OF_IRQ, so > it needs to be moved out of that as well. > > On Sparc, CONFIG_OF is true, but CONFIG_OF_IRQ is false. So that > combination needs to be handled as well. OK, I'm travelling for a couple of days now, but I'll respin something when I'm back. I guess it's a case of always exporting the simple ops when CONFIG_IRQDOMAIN=y, and only setting the .dt_translate when CONFIG_OF_IRQ=y though. Jamie ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq 2011-11-10 10:53 [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq Jamie Iles 2011-11-10 11:14 ` Jamie Iles @ 2011-11-16 11:34 ` Jamie Iles 1 sibling, 0 replies; 8+ messages in thread From: Jamie Iles @ 2011-11-16 11:34 UTC (permalink / raw) To: Jamie Iles; +Cc: linux-kernel, Thomas Gleixner, Grant Likely, Rob Herring On Thu, Nov 10, 2011 at 10:53:03AM +0000, Jamie Iles wrote: > It is optional for an irqdomain to have a to_irq() method, and for > simple domains they often don't require any operations at all - just > hwirq to Linux irq translation. Check we have valid ops before > dereferencing them. > > Patch originally by Rob Herring. > > Suggested-by: Rob Herring <robherring2@gmail.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Grant Likely <grant.likely@secretlab.ca> > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > --- > > Rob, I can't see that you've already posted this but I didn't want to > hold Marc's GIC patches up. > > include/linux/irqdomain.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > index 99834e58..78a1e66 100644 > --- a/include/linux/irqdomain.h > +++ b/include/linux/irqdomain.h > @@ -74,7 +74,7 @@ struct irq_domain { > static inline unsigned int irq_domain_to_irq(struct irq_domain *d, > unsigned long hwirq) > { > - if (d->ops->to_irq) > + if (d->ops && d->ops->to_irq) > return d->ops->to_irq(d, hwirq); > if (WARN_ON(hwirq < d->hwirq_base)) > return 0; > -- ping? Marc has sent a pull request for the VIC/GIC consolidation patches and these really need this one. Thanks, Jamie ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-11-27 15:24 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-10 10:53 [PATCH] irq: fix possible null-pointer deref irq_domain_to_irq Jamie Iles 2011-11-10 11:14 ` Jamie Iles 2011-11-10 13:37 ` Rob Herring 2011-11-23 12:23 ` Jamie Iles 2011-11-25 15:35 ` Jamie Iles 2011-11-27 15:11 ` Rob Herring 2011-11-27 15:24 ` Jamie Iles 2011-11-16 11:34 ` Jamie Iles
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox