hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IBHTNK -------------------------------- The current mechanism for printing dirty pages involves "/proc/dirty/buffer_size" and "/proc/dirty/dirty_list" being able to be opened simultaneously. If "buffer_size" is opened firstly and 1 is written to it, and then "dirty_list" is opened, followed by writing 0 to "buffer_size". Then variable "buf_dirty" will be freed and set to NULL. However, the user can still access "buf_dirty" through the already opened "dirty_list", which could lead to a NULL pointer dereference issue. To prevent the NULL pointer dereference problem, add an exclusive open lock. And now "/proc/dirty/buffer_size" and "/proc/dirty/dirty_list" cannot be opened at the same time unless "/proc/dirty/buffer_size" is opened in read-only mode. At the same time, "buff_used" and "buff_lock" in the original code are not needed, and the redundant code is deleted. Fixes: aeb96447becc ("fs/dirty_pages: dump the number of dirty pages for each inode") Signed-off-by: Zizhi Wo <wozizhi@huawei.com> --- fs/dirty_pages.c | 83 +++++++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/fs/dirty_pages.c b/fs/dirty_pages.c index f75a5158d94b..81bd52ad50f0 100644 --- a/fs/dirty_pages.c +++ b/fs/dirty_pages.c @@ -19,12 +19,10 @@ static char *buf_dirty; /* buffer to store number of dirty pages */ static unsigned long buf_size; /* size of buffer in bytes */ static long buff_num; /* size of buffer in number of pages */ static int buff_limit; /* filter threshold of dirty pages*/ +static unsigned long lock_word; /* for exclusive access to buffer */ static struct proc_dir_entry *dirty_dir; -static struct mutex buff_used; /* buffer is in used */ -static struct mutex buff_lock; /* lock when buffer is changed */ - /* proc root directory */ #define DIRTY_ROOT "dirty" /* proc file for buffer allocation and release */ @@ -40,6 +38,18 @@ static void seq_set_overflow(struct seq_file *m) m->count = m->size; } +static bool dirty_pages_lock(void) +{ + if (xchg(&lock_word, 1) == 1) + return false; + return true; +} + +static void dirty_pages_unlock(void) +{ + lock_word = 0; +} + static unsigned long dump_dirtypages_inode(struct inode *inode) { struct pagevec pvec; @@ -195,10 +205,6 @@ static ssize_t seq_read_dirty( size_t n; int err = 0; - if (!mutex_trylock(&buff_used)) - return -EBUSY; - /* don't allow buffer to change during read */ - mutex_lock(&buff_lock); if (m->count == 0) { memset(buf_dirty, 0, buf_size); if (!m->buf) { @@ -210,13 +216,6 @@ static ssize_t seq_read_dirty( goto done; } - /* if buffer changed somehow */ - if (m->size != buf_size) { - mutex_unlock(&buff_lock); - mutex_unlock(&buff_used); - return -EFAULT; - } - n = min(m->count - m->from, size); /* check if this is the last read */ if (n == 0) @@ -234,8 +233,6 @@ static ssize_t seq_read_dirty( else *ppos += copied; - mutex_unlock(&buff_lock); - mutex_unlock(&buff_used); return copied; } @@ -247,6 +244,7 @@ static void free_buf_dirty(void) buf_size = 0; } } + static ssize_t write_proc( struct file *filp, const char *buf, @@ -257,9 +255,6 @@ static ssize_t write_proc( int ret = 0; long old_buff_num; - if (!mutex_trylock(&buff_used)) - return -EBUSY; - if (count > PAGE_SIZE) { ret = -EINVAL; goto error; @@ -283,42 +278,44 @@ static ssize_t write_proc( goto free; } - mutex_lock(&buff_lock); - ret = count; if (buff_num == 0) { free_buf_dirty(); - goto out; + goto free; } if (buff_num == old_buff_num) - goto out; + goto free; free_buf_dirty(); buf_size = PAGE_SIZE * buff_num; buf_dirty = vzalloc(buf_size); - if (!buf_dirty) { + if (!buf_dirty) ret = -ENOMEM; - goto out; - } -out: - mutex_unlock(&buff_lock); free: kfree(msg); error: - mutex_unlock(&buff_used); return ret; } static int proc_dpages_open(struct inode *inode, struct file *filp) { + int ret; + + if (!dirty_pages_lock()) + return -EBUSY; + if (buf_dirty == NULL || buf_size == 0) { pr_warn("please allocate buffer before getting dirty pages\n"); + dirty_pages_unlock(); return -ENOMEM; } - return single_open(filp, proc_dpages_show, NULL); + ret = single_open(filp, proc_dpages_show, NULL); + if (ret) + dirty_pages_unlock(); + return ret; } static int seq_release_dirty(struct inode *inode, struct file *file) @@ -327,6 +324,7 @@ static int seq_release_dirty(struct inode *inode, struct file *file) /* we don't want to free the buf */ m->buf = NULL; + dirty_pages_unlock(); single_release(inode, file); return 0; } @@ -351,7 +349,16 @@ static int proc_limit_show(struct seq_file *m, void *v) static int proc_switch_open(struct inode *inode, struct file *filp) { - return single_open(filp, proc_switch_show, NULL); + int ret; + + if (((filp->f_flags & O_ACCMODE) != O_RDONLY) && !dirty_pages_lock()) + return -EBUSY; + + ret = single_open(filp, proc_switch_show, NULL); + if (ret && ((filp->f_flags & O_ACCMODE) != O_RDONLY)) + dirty_pages_unlock(); + + return ret; } static int proc_limit_open(struct inode *inode, struct file *filp) @@ -359,6 +366,14 @@ static int proc_limit_open(struct inode *inode, struct file *filp) return single_open(filp, proc_limit_show, NULL); } +static int proc_switch_release(struct inode *inode, struct file *filp) +{ + if ((filp->f_flags & O_ACCMODE) != O_RDONLY) + dirty_pages_unlock(); + + return single_release(inode, filp); +} + static ssize_t write_limit_proc( struct file *filp, const char *buf, @@ -405,7 +420,7 @@ static const struct proc_ops proc_switch_operations = { .proc_read = seq_read, .proc_write = write_proc, .proc_lseek = seq_lseek, - .proc_release = single_release, + .proc_release = proc_switch_release, }; static const struct proc_ops proc_limit_operations = { @@ -440,8 +455,6 @@ static int __init dpages_proc_init(void) if (!proc_file) goto fail_limit; - mutex_init(&buff_used); - mutex_init(&buff_lock); return 0; fail_limit: @@ -456,9 +469,7 @@ static int __init dpages_proc_init(void) static void dpages_proc_exit(void) { - mutex_lock(&buff_lock); free_buf_dirty(); - mutex_unlock(&buff_lock); remove_proc_entry(DIRTY_PAGES, dirty_dir); remove_proc_entry(DIRTY_SWITCH, dirty_dir); remove_proc_entry(DIRTY_LIMIT, dirty_dir); -- 2.39.2