From: yu kuai yukuai3@huawei.com
euler inclusion category: bugfix bugzilla: 46858 CVE: NA
---------------------------
In write_proc and write_limit_proc, we alloccate 'msg' with 'PAGE_SIZE' bytes, which is defined as 4096. The problem is that if 'count' is 4096, the following code will cause index out of bounds:
msg[count] = '\0'
In order to fix the problem, we use kzalloc instead of kmalloc and delete the code above.
'buff_limit' is static int type, but 'temp' is log type in write_limit_proc. Thus if we input a value which is more than 'MAX_INT' (the max integer of int type), we can get a negative number for 'buff_limit'. For example:
echo 2147483648 > /proc/dirty/page_threshold cat /proc/dirty/page_threshold -2147483648
Fix the problem by changing 'temp < 0' to 'tmp < 0 || temp > MAX_INT'
Fixes: 3296069cbce1 ("fs/dirty_pages: dump the number of dirty pages for each inode") Reported-by: song jian songjian15@huawei.com Signed-off-by: yu kuai yukuai3@huawei.com Reviewed-by: zhangyi (F) yi.zhang@huawei.com Signed-off-by: zhangyi (F) yi.zhang@huawei.com Signed-off-by: Dianfang Zhang zhangdianfang@huawei.com --- fs/dirty_pages.c | 45 ++++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/fs/dirty_pages.c b/fs/dirty_pages.c index 9972b53d9acc..7a32f01021fb 100644 --- a/fs/dirty_pages.c +++ b/fs/dirty_pages.c @@ -36,6 +36,7 @@ static struct mutex buff_lock; /* lock when buffer is changed */ /* proc file to filter result */ #define DIRTY_LIMIT "page_threshold"
+#define MAX_BUFF_SIZE 102400 static void seq_set_overflow(struct seq_file *m) { m->count = m->size; @@ -240,28 +241,27 @@ static ssize_t write_proc( int ret = 0; long old_buff_num;
- msg = kmalloc(PAGE_SIZE, GFP_KERNEL); - if (!msg) - return -ENOMEM; - if (count > PAGE_SIZE) { ret = -EINVAL; goto error; }
- msg[count] = '\0'; + msg = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!msg) { + ret = -ENOMEM; + goto error; + }
if (copy_from_user(msg, buf, count)) { ret = -EINVAL; - goto error; + goto free; } - old_buff_num = buff_num; ret = kstrtol(msg, 10, &buff_num); - if (ret != 0 || buff_num < 0 || buff_num > 102400) { + if (ret != 0 || buff_num < 0 || buff_num > MAX_BUFF_SIZE) { buff_num = 0; ret = -EINVAL; - goto error; + goto free; }
mutex_lock(&buff_lock); @@ -292,8 +292,9 @@ static ssize_t write_proc( out: buff_used = false; mutex_unlock(&buff_lock); -error: +free: kfree(msg); +error: return ret; }
@@ -377,31 +378,33 @@ static ssize_t write_limit_proc( int ret = 0; long temp;
- msg = kmalloc(PAGE_SIZE, GFP_KERNEL); - if (!msg) - return -ENOMEM; - if (count > PAGE_SIZE) { ret = -EINVAL; goto error; }
- msg[count] = '\0'; + msg = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!msg) { + ret = -ENOMEM; + goto error; + } + if (copy_from_user(msg, buf, count)) { ret = -EINVAL; - goto error; + goto free; } ret = kstrtol(msg, 10, &temp); - if (ret != 0 || temp < 0) { - ret = -EINVAL; - goto error; -} + if (ret != 0 || temp < 0 || temp > INT_MAX) { + ret = -EINVAL; + goto free; + }
WRITE_ONCE(buff_limit, temp); ret = count;
-error: +free: kfree(msg); +error: return ret; }