Commit 98bc93e505c03403479c6669c4ff97301cee6199
Committed by
Linus Torvalds
Linus Torvalds
1 parent
30cd890391
Exists in
master
and in
20 other branches
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>
Showing 1 changed file with 9 additions and 10 deletions Side-by-side Diff
fs/proc/task_mmu.c
| ... | ... | @@ -771,18 +771,12 @@ |
| 771 | 771 | if (!task) |
| 772 | 772 | goto out; |
| 773 | 773 | |
| 774 | - mm = mm_for_maps(task); | |
| 775 | - ret = PTR_ERR(mm); | |
| 776 | - if (!mm || IS_ERR(mm)) | |
| 777 | - goto out_task; | |
| 778 | - | |
| 779 | 774 | ret = -EINVAL; |
| 780 | 775 | /* file position must be aligned */ |
| 781 | 776 | if ((*ppos % PM_ENTRY_BYTES) || (count % PM_ENTRY_BYTES)) |
| 782 | 777 | goto out_task; |
| 783 | 778 | |
| 784 | 779 | ret = 0; |
| 785 | - | |
| 786 | 780 | if (!count) |
| 787 | 781 | goto out_task; |
| 788 | 782 | |
| 789 | 783 | |
| ... | ... | @@ -790,8 +784,13 @@ |
| 790 | 784 | pm.buffer = kmalloc(pm.len, GFP_TEMPORARY); |
| 791 | 785 | ret = -ENOMEM; |
| 792 | 786 | if (!pm.buffer) |
| 793 | - goto out_mm; | |
| 787 | + goto out_task; | |
| 794 | 788 | |
| 789 | + mm = mm_for_maps(task); | |
| 790 | + ret = PTR_ERR(mm); | |
| 791 | + if (!mm || IS_ERR(mm)) | |
| 792 | + goto out_free; | |
| 793 | + | |
| 795 | 794 | pagemap_walk.pmd_entry = pagemap_pte_range; |
| 796 | 795 | pagemap_walk.pte_hole = pagemap_pte_hole; |
| 797 | 796 | #ifdef CONFIG_HUGETLB_PAGE |
| ... | ... | @@ -833,7 +832,7 @@ |
| 833 | 832 | len = min(count, PM_ENTRY_BYTES * pm.pos); |
| 834 | 833 | if (copy_to_user(buf, pm.buffer, len)) { |
| 835 | 834 | ret = -EFAULT; |
| 836 | - goto out_free; | |
| 835 | + goto out_mm; | |
| 837 | 836 | } |
| 838 | 837 | copied += len; |
| 839 | 838 | buf += len; |
| 840 | 839 | |
| ... | ... | @@ -843,10 +842,10 @@ |
| 843 | 842 | if (!ret || ret == PM_END_OF_BUFFER) |
| 844 | 843 | ret = copied; |
| 845 | 844 | |
| 846 | -out_free: | |
| 847 | - kfree(pm.buffer); | |
| 848 | 845 | out_mm: |
| 849 | 846 | mmput(mm); |
| 847 | +out_free: | |
| 848 | + kfree(pm.buffer); | |
| 850 | 849 | out_task: |
| 851 | 850 | put_task_struct(task); |
| 852 | 851 | out: |