Skip to content

Commit 98bc93e

Browse files
kosakitorvalds
authored andcommitted
proc: fix pagemap_read() error case
Currently, pagemap_read() has three error and/or corner case handling mistake. (1) If ppos parameter is wrong, mm refcount will be leak. (2) If count parameter is 0, mm refcount will be leak too. (3) If the current task is sleeping in kmalloc() and the system is out of memory and oom-killer kill the proc associated task, mm_refcount prevent the task free its memory. then system may hang up. <Quote Hugh's explain why we shold call kmalloc() before get_mm()> 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. This patch fixes the above three. Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Cc: Hugh Dickins <hughd@google.com> Cc: Jovi Zhang <bookjovi@gmail.com> Acked-by: Hugh Dickins <hughd@google.com> Cc: 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 30cd890 commit 98bc93e

File tree

1 file changed

+9
-10
lines changed

1 file changed

+9
-10
lines changed

fs/proc/task_mmu.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -771,26 +771,25 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
771771
if (!task)
772772
goto out;
773773

774-
mm = mm_for_maps(task);
775-
ret = PTR_ERR(mm);
776-
if (!mm || IS_ERR(mm))
777-
goto out_task;
778-
779774
ret = -EINVAL;
780775
/* file position must be aligned */
781776
if ((*ppos % PM_ENTRY_BYTES) || (count % PM_ENTRY_BYTES))
782777
goto out_task;
783778

784779
ret = 0;
785-
786780
if (!count)
787781
goto out_task;
788782

789783
pm.len = PM_ENTRY_BYTES * (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
790784
pm.buffer = kmalloc(pm.len, GFP_TEMPORARY);
791785
ret = -ENOMEM;
792786
if (!pm.buffer)
793-
goto out_mm;
787+
goto out_task;
788+
789+
mm = mm_for_maps(task);
790+
ret = PTR_ERR(mm);
791+
if (!mm || IS_ERR(mm))
792+
goto out_free;
794793

795794
pagemap_walk.pmd_entry = pagemap_pte_range;
796795
pagemap_walk.pte_hole = pagemap_pte_hole;
@@ -833,7 +832,7 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
833832
len = min(count, PM_ENTRY_BYTES * pm.pos);
834833
if (copy_to_user(buf, pm.buffer, len)) {
835834
ret = -EFAULT;
836-
goto out_free;
835+
goto out_mm;
837836
}
838837
copied += len;
839838
buf += len;
@@ -843,10 +842,10 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
843842
if (!ret || ret == PM_END_OF_BUFFER)
844843
ret = copied;
845844

846-
out_free:
847-
kfree(pm.buffer);
848845
out_mm:
849846
mmput(mm);
847+
out_free:
848+
kfree(pm.buffer);
850849
out_task:
851850
put_task_struct(task);
852851
out:

0 commit comments

Comments
 (0)