linux-kernel - [tip:irq/urgent] irqdesc: Use bool return type instead of int

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]

Date:   Mon, 4 Dec 2017 11:58:20 -0800
From:   tip-bot for Will Deacon <tipbot@...or.com>
To:     linux-tip-commits@...r.kernel.org
Cc:     torvalds@...ux-foundation.org, tglx@...utronix.de,
        will.deacon@....com, linux-kernel@...r.kernel.org, hpa@...or.com,
        mingo@...nel.org
Subject: [tip:irq/urgent] irqdesc: Use bool return type instead of int

Commit-ID:  4ce413d1840b25b101be3c0559161db8891f3360
Gitweb:     https://git.kernel.org/tip/4ce413d1840b25b101be3c0559161db8891f3360
Author:     Will Deacon <will.deacon@....com>
AuthorDate: Fri, 1 Dec 2017 15:29:39 +0000
Committer:  Thomas Gleixner <tglx@...utronix.de>
CommitDate: Mon, 4 Dec 2017 20:51:12 +0100

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 <torvalds@...ux-foundation.org>
Signed-off-by: Will Deacon <will.deacon@....com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Cc: marc.zyngier@....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 dd41895..39fb370 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;