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@linaro.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))