Skip to content

Commit 30cd890

Browse files
kosakitorvalds
authored andcommitted
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 <bookjovi@gmail.com> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Acked-by: Hugh Dickins <hughd@google.com> Reviewed-by: Stephen Wilson <wilsons@start.ca> Cc: Alexey Dobriyan <adobriyan@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent a4dbf0e commit 30cd890

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

fs/proc/base.c

+9-7
Original file line numberDiff line numberDiff line change
@@ -894,20 +894,20 @@ static ssize_t mem_write(struct file * file, const char __user *buf,
894894
if (!task)
895895
goto out_no_task;
896896

897+
copied = -ENOMEM;
898+
page = (char *)__get_free_page(GFP_TEMPORARY);
899+
if (!page)
900+
goto out_task;
901+
897902
mm = check_mem_permission(task);
898903
copied = PTR_ERR(mm);
899904
if (IS_ERR(mm))
900-
goto out_task;
905+
goto out_free;
901906

902907
copied = -EIO;
903908
if (file->private_data != (void *)((long)current->self_exec_id))
904909
goto out_mm;
905910

906-
copied = -ENOMEM;
907-
page = (char *)__get_free_page(GFP_TEMPORARY);
908-
if (!page)
909-
goto out_mm;
910-
911911
copied = 0;
912912
while (count > 0) {
913913
int this_len, retval;
@@ -929,9 +929,11 @@ static ssize_t mem_write(struct file * file, const char __user *buf,
929929
count -= retval;
930930
}
931931
*ppos = dst;
932-
free_page((unsigned long) page);
932+
933933
out_mm:
934934
mmput(mm);
935+
out_free:
936+
free_page((unsigned long) page);
935937
out_task:
936938
put_task_struct(task);
937939
out_no_task:

0 commit comments

Comments
 (0)