From 30cd8903913dac7b0918807cac46be3ecde5a5a7 Mon Sep 17 00:00:00 2001 From: KOSAKI Motohiro Date: Thu, 26 May 2011 16:25:52 -0700 Subject: [PATCH] proc: put check_mem_permission after __get_free_page in mem_write It whould be better if put check_mem_permission after __get_free_page in mem_write, to be same as function mem_read. Hugh Dickins explained the reason. check_mem_permission gets a reference to the mm. If we __get_free_page after check_mem_permission, imagine what happens if the system is out of memory, and the mm we're looking at is selected for killing by the OOM killer: while we wait in __get_free_page for more memory, no memory is freed from the selected mm because it cannot reach exit_mmap while we hold that reference. Reported-by: Jovi Zhang Signed-off-by: KOSAKI Motohiro Acked-by: Hugh Dickins Reviewed-by: Stephen Wilson Cc: Alexey Dobriyan Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/proc/base.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 0c2c50cc2cca..4ede550517a6 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -894,20 +894,20 @@ static ssize_t mem_write(struct file * file, const char __user *buf, if (!task) goto out_no_task; + copied = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); + if (!page) + goto out_task; + mm = check_mem_permission(task); copied = PTR_ERR(mm); if (IS_ERR(mm)) - goto out_task; + goto out_free; copied = -EIO; if (file->private_data != (void *)((long)current->self_exec_id)) goto out_mm; - copied = -ENOMEM; - page = (char *)__get_free_page(GFP_TEMPORARY); - if (!page) - goto out_mm; - copied = 0; while (count > 0) { int this_len, retval; @@ -929,9 +929,11 @@ static ssize_t mem_write(struct file * file, const char __user *buf, count -= retval; } *ppos = dst; - free_page((unsigned long) page); + out_mm: mmput(mm); +out_free: + free_page((unsigned long) page); out_task: put_task_struct(task); out_no_task: -- 2.11.0