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 | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 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,18 +894,18 @@ static ssize_t mem_write(struct file * file, const char __user *buf, if (!task) goto out_no_task; - mm = check_mem_permission(task); - copied = PTR_ERR(mm); - if (IS_ERR(mm)) - goto out_task; - - 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_task; + + mm = check_mem_permission(task); + copied = PTR_ERR(mm); + if (IS_ERR(mm)) + goto out_free; + + copied = -EIO; + if (file->private_data != (void *)((long)current->self_exec_id)) goto out_mm; copied = 0; @@ -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: