linux-kernel - [PATCH] debugfs: don't access 4 bytes for a boolean

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

Message-Id: <3d6f65fa15363650f2d10ca58b9d9d243e98980f.1441961769.git.viresh.kumar@linaro.org>
Date:	Fri, 11 Sep 2015 14:36:06 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	gregkh@...uxfoundation.org
Cc:	linaro-kernel@...ts.linaro.org, Rafael Wysocki <rjw@...ysocki.net>,
	sboyd@...eaurora.org, Viresh Kumar <viresh.kumar@...aro.org>,
	linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] debugfs: don't access 4 bytes for a boolean

Long back 'bool' type used to be a typecast to 'int', but that changed
in v2.6.19. And that is a typecast to _Bool now, which (mostly) takes
just a byte. Anyway, the bool type in kernel is used to store true/false
or 1/0 only. So, accessing a single byte should be enough.

The problem with current code is that it reads/writes 4 bytes for a
boolean, which will read/update 3 excess bytes following the boolean
variable. And that can lead to hard to fix bugs. It was a nightmare to
crack this one.

The debugfs code had this bug since the first time it got introduced,
but was never got caught, strange. Maybe the bool variables (monitored
by debugfs) were followed by an 'int' or something bigger and the pad
bytes made sure, we never see this issue.

But the OPP (Operating performance points) library have three booleans
allocated to contiguous bytes and this bug got hit quite soon (The
debugfs support for OPP is yet to be merged).

Fix this by changing type of 'val' pointer to u8 type, so that we only
access a single byte.

Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
---
Greg,

I wasn't sure about what to add the stable tag. This bug is around for a
really long time now.

And this also gets me worrying if any other part of the kernel are
treating booleans in a similar way :)

Also, there is another problem I see, which probably should be fixed as
well. But I wanted to hear from you before trying to patch the kernel
for this.

debugfs_create_bool() declares the pointer to be of type u32 *.
Shouldn't that be changed to u8 *? There are many users which are
typecasting the variables to make debugfs API happy :)

 fs/debugfs/file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 284f9aa0028b..c123185a296a 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -439,7 +439,7 @@ static ssize_t read_file_bool(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
 {
 	char buf[3];
-	u32 *val = file->private_data;
+	u8 *val = file->private_data;

 	if (*val)
 		buf[0] = 'Y';
@@ -456,7 +456,7 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf,
 	char buf[32];
 	size_t buf_size;
 	bool bv;
-	u32 *val = file->private_data;
+	u8 *val = file->private_data;

 	buf_size = min(count, (sizeof(buf)-1));
 	if (copy_from_user(buf, user_buf, buf_size))
-- 
2.4.0

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