From bb5c43428252f27b875c764451321a83a531d6e5 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 28 Nov 2017 15:40:33 +0100 Subject: [PATCH 1/2] genirq/matrix: Fix the precedence fix for real The previous commit which made the operator precedence in irq_matrix_available() explicit made the implicit brokenness explicitely wrong. It was wrong in the original commit already. The overworked maintainer did not notice it either when merging the patch. Replace the confusing '?' construct by a simple and obvious if (). Fixes: 75f1133873d6 ("genirq/matrix: Make - vs ?: Precedence explicit") Reported-by: Rasmus Villemoes Signed-off-by: Thomas Gleixner Cc: Kees Cook --- kernel/irq/matrix.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c index 7df2480005f8..0ba0dd8863a7 100644 --- a/kernel/irq/matrix.c +++ b/kernel/irq/matrix.c @@ -384,7 +384,9 @@ unsigned int irq_matrix_available(struct irq_matrix *m, bool cpudown) { struct cpumap *cm = this_cpu_ptr(m->maps); - return (m->global_available - cpudown) ? cm->available : 0; + if (!cpudown) + return m->global_available; + return m->global_available - cm->available; } /** From 4ce413d1840b25b101be3c0559161db8891f3360 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Fri, 1 Dec 2017 15:29:39 +0000 Subject: [PATCH 2/2] irqdesc: Use bool return type instead of int The irq_balancing_disabled and irq_is_percpu{,_devid} functions are clearly intended to return bool like the functions in kernel/irq/settings.h, but actually return an int containing a masked value of desc->status_use_accessors. This can lead to subtle breakage if, for example, the return value is subsequently truncated when assigned to a narrower type. As Linus points out: | In particular, what can (and _has_ happened) is that people end up | using these functions that return true or false, and they assign the | result to something like a bitfield (or a char) or whatever. | | And the code looks *obviously* correct, when you have things like | | dev->percpu = irq_is_percpu_devid(dev->irq); | | and that "percpu" thing is just one status bit among many. It may even | *work*, because maybe that "percpu" flag ends up not being all that | important, or it just happens to never be set on the particular | hardware that people end up testing. | | But while it looks obviously correct, and might even work, it's really | fundamentally broken. Because that "true or false" function didn't | actually return 0/1, it returned 0 or 0x20000. | | And 0x20000 may not fit in a bitmask or a "char" or whatever. Fix the problem by consistently using bool as the return type for these functions. Reported-by: Linus Torvalds Signed-off-by: Will Deacon Signed-off-by: Thomas Gleixner Cc: marc.zyngier@arm.com Link: https://lkml.kernel.org/r/1512142179-24616-1-git-send-email-will.deacon@arm.com --- include/linux/irqdesc.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index dd418955962b..39fb3700f7a9 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -230,7 +230,7 @@ irq_set_chip_handler_name_locked(struct irq_data *data, struct irq_chip *chip, data->chip = chip; } -static inline int irq_balancing_disabled(unsigned int irq) +static inline bool irq_balancing_disabled(unsigned int irq) { struct irq_desc *desc; @@ -238,7 +238,7 @@ static inline int irq_balancing_disabled(unsigned int irq) return desc->status_use_accessors & IRQ_NO_BALANCING_MASK; } -static inline int irq_is_percpu(unsigned int irq) +static inline bool irq_is_percpu(unsigned int irq) { struct irq_desc *desc; @@ -246,7 +246,7 @@ static inline int irq_is_percpu(unsigned int irq) return desc->status_use_accessors & IRQ_PER_CPU; } -static inline int irq_is_percpu_devid(unsigned int irq) +static inline bool irq_is_percpu_devid(unsigned int irq) { struct irq_desc *desc;