hulk inclusion category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/10228 -------------------------------- proc_buffer_open() unconditionally releases the lock on its error path, even when it did not acquire the lock itself. This leads to a UAF issue. Consider the following sequence: 1. proc_dpages_open() acquires the lock and sets m->buf = buf_dirty. 2. proc_buffer_open() fails and, on its error path, releases the lock that was actually taken by proc_dpages_open(). 3. With the lock now wrongly released, userspace calls proc_buffer_open() and proc_buffer_write() again, re-acquires the lock and frees "buf_dirty" via free_buf_dirty(). 4. At this point m->buf still points to the already-freed buf_dirty. Any subsequent read path then dereferences m->buf and triggers a UAF. Fix this by pairing lock acquisition and release so that a function only releases the lock it has actually acquired. Fixes: 869921da6eaa ("fs/dirty_pages: dump the number of dirty pages for each inode") Signed-off-by: Zizhi Wo <wozizhi@huawei.com> --- fs/dirty_pages.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/dirty_pages.c b/fs/dirty_pages.c index 712992cba9e6..1a50420fd0f1 100644 --- a/fs/dirty_pages.c +++ b/fs/dirty_pages.c @@ -223,16 +223,17 @@ static int proc_buffer_show(struct seq_file *m, void *v) } static int proc_buffer_open(struct inode *inode, struct file *filp) { int ret; + bool need_lock = (filp->f_flags & O_ACCMODE) != O_RDONLY; - if (((filp->f_flags & O_ACCMODE) != O_RDONLY) && !dirty_pages_lock()) + if (need_lock && !dirty_pages_lock()) return -EBUSY; ret = single_open(filp, proc_buffer_show, NULL); - if (ret) + if (ret && need_lock) dirty_pages_unlock(); return ret; } -- 2.52.0