| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 2 Sep 2021 17:28:59 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Colin Cross <ccross@...gle.com>,
Sumit Semwal <sumit.semwal@...aro.org>,
Michal Hocko <mhocko@...e.com>,
Dave Hansen <dave.hansen@...el.com>,
Kees Cook <keescook@...omium.org>,
Matthew Wilcox <willy@...radead.org>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
Vlastimil Babka <vbabka@...e.cz>,
Johannes Weiner <hannes@...xchg.org>,
Jonathan Corbet <corbet@....net>,
Al Viro <viro@...iv.linux.org.uk>,
Randy Dunlap <rdunlap@...radead.org>,
Kalesh Singh <kaleshsingh@...gle.com>,
Peter Xu <peterx@...hat.com>, rppt@...nel.org,
Peter Zijlstra <peterz@...radead.org>,
Catalin Marinas <catalin.marinas@....com>,
vincenzo.frascino@....com,
Chinwen Chang (張錦文)
<chinwen.chang@...iatek.com>,
Axel Rasmussen <axelrasmussen@...gle.com>,
Andrea Arcangeli <aarcange@...hat.com>,
Jann Horn <jannh@...gle.com>, apopple@...dia.com,
John Hubbard <jhubbard@...dia.com>,
Yu Zhao <yuzhao@...gle.com>, Will Deacon <will@...nel.org>,
fenghua.yu@...el.com, thunder.leizhen@...wei.com,
Hugh Dickins <hughd@...gle.com>, feng.tang@...el.com,
Jason Gunthorpe <jgg@...pe.ca>, Roman Gushchin <guro@...com>,
Thomas Gleixner <tglx@...utronix.de>, krisman@...labora.com,
chris.hyser@...cle.com, Peter Collingbourne <pcc@...gle.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Jens Axboe <axboe@...nel.dk>, legion@...nel.org,
Rolf Eike Beer <eb@...ix.com>,
Cyrill Gorcunov <gorcunov@...il.com>,
Muchun Song <songmuchun@...edance.com>,
Viresh Kumar <viresh.kumar@...aro.org>,
Thomas Cedeno <thomascedeno@...gle.com>, sashal@...nel.org,
cxfcosmos@...il.com, Rasmus Villemoes <linux@...musvillemoes.dk>,
LKML <linux-kernel@...r.kernel.org>,
linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-mm <linux-mm@...ck.org>,
kernel-team <kernel-team@...roid.com>,
Pekka Enberg <penberg@...nel.org>,
Ingo Molnar <mingo@...nel.org>,
Oleg Nesterov <oleg@...hat.com>,
Jan Glauber <jan.glauber@...il.com>,
John Stultz <john.stultz@...aro.org>,
Rob Landley <rob@...dley.net>,
Cyrill Gorcunov <gorcunov@...nvz.org>,
"Serge E. Hallyn" <serge.hallyn@...ntu.com>,
David Rientjes <rientjes@...gle.com>,
Mel Gorman <mgorman@...e.de>, Shaohua Li <shli@...ionio.com>,
Minchan Kim <minchan@...nel.org>
Subject: Re: [PATCH v9 1/3] mm: rearrange madvise code to allow for reuse
On Thu, Sep 2, 2021 at 4:18 PM Suren Baghdasaryan <surenb@...gle.com> wrote:
>
> From: Colin Cross <ccross@...gle.com>
>
> Refactor the madvise syscall to allow for parts of it to be reused by a
> prctl syscall that affects vmas.
>
> Move the code that walks vmas in a virtual address range into a function
> that takes a function pointer as a parameter. The only caller for now is
> sys_madvise, which uses it to call madvise_vma_behavior on each vma, but
> the next patch will add an additional caller.
>
> Move handling all vma behaviors inside madvise_behavior, and rename it to
> madvise_vma_behavior.
>
> Move the code that updates the flags on a vma, including splitting or
> merging the vma as necessary, into a new function called
> madvise_update_vma. The next patch will add support for updating a new
> anon_name field as well.
>
> Signed-off-by: Colin Cross <ccross@...gle.com>
> Cc: Pekka Enberg <penberg@...nel.org>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Oleg Nesterov <oleg@...hat.com>
> Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
> Cc: Jan Glauber <jan.glauber@...il.com>
> Cc: John Stultz <john.stultz@...aro.org>
> Cc: Rob Landley <rob@...dley.net>
> Cc: Cyrill Gorcunov <gorcunov@...nvz.org>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: "Serge E. Hallyn" <serge.hallyn@...ntu.com>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: Al Viro <viro@...iv.linux.org.uk>
> Cc: Hugh Dickins <hughd@...gle.com>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Shaohua Li <shli@...ionio.com>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: Minchan Kim <minchan@...nel.org>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> [sumits: rebased over v5.9-rc3]
> Signed-off-by: Sumit Semwal <sumit.semwal@...aro.org>
> [surenb: rebased over v5.14-rc7]
> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> ---
> previous version including cover letter with test results is at:
> https://lore.kernel.org/linux-mm/20210827191858.2037087-1-surenb@google.com/
>
> changes in v9
> - Removed unnecessary initialization of 'error' to 0 in madvise_vma_behavior,
> per Cyrill Gorcunov
> - Replaced goto's with returns in madvise_vma_behavior, per Cyrill Gorcunov
> - Recovered the comment explaining why we map ENOMEM to EAGAIN in
> madvise_vma_behavior, per Cyrill Gorcunov
>
> mm/madvise.c | 317 +++++++++++++++++++++++++++------------------------
> 1 file changed, 170 insertions(+), 147 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 56324a3dbc4e..54bf9f73f95d 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -63,76 +63,20 @@ static int madvise_need_mmap_write(int behavior)
> }
>
> /*
> - * We can potentially split a vm area into separate
> - * areas, each area with its own behavior.
> + * Update the vm_flags on regiion of a vma, splitting it or merging it as
> + * necessary. Must be called with mmap_sem held for writing;
> */
> -static long madvise_behavior(struct vm_area_struct *vma,
> - struct vm_area_struct **prev,
> - unsigned long start, unsigned long end, int behavior)
> +static int madvise_update_vma(struct vm_area_struct *vma,
> + struct vm_area_struct **prev, unsigned long start,
> + unsigned long end, unsigned long new_flags)
> {
> struct mm_struct *mm = vma->vm_mm;
> - int error = 0;
> + int error;
> pgoff_t pgoff;
> - unsigned long new_flags = vma->vm_flags;
> -
> - switch (behavior) {
> - case MADV_NORMAL:
> - new_flags = new_flags & ~VM_RAND_READ & ~VM_SEQ_READ;
> - break;
> - case MADV_SEQUENTIAL:
> - new_flags = (new_flags & ~VM_RAND_READ) | VM_SEQ_READ;
> - break;
> - case MADV_RANDOM:
> - new_flags = (new_flags & ~VM_SEQ_READ) | VM_RAND_READ;
> - break;
> - case MADV_DONTFORK:
> - new_flags |= VM_DONTCOPY;
> - break;
> - case MADV_DOFORK:
> - if (vma->vm_flags & VM_IO) {
> - error = -EINVAL;
> - goto out;
> - }
> - new_flags &= ~VM_DONTCOPY;
> - break;
> - case MADV_WIPEONFORK:
> - /* MADV_WIPEONFORK is only supported on anonymous memory. */
> - if (vma->vm_file || vma->vm_flags & VM_SHARED) {
> - error = -EINVAL;
> - goto out;
> - }
> - new_flags |= VM_WIPEONFORK;
> - break;
> - case MADV_KEEPONFORK:
> - new_flags &= ~VM_WIPEONFORK;
> - break;
> - case MADV_DONTDUMP:
> - new_flags |= VM_DONTDUMP;
> - break;
> - case MADV_DODUMP:
> - if (!is_vm_hugetlb_page(vma) && new_flags & VM_SPECIAL) {
> - error = -EINVAL;
> - goto out;
> - }
> - new_flags &= ~VM_DONTDUMP;
> - break;
> - case MADV_MERGEABLE:
> - case MADV_UNMERGEABLE:
> - error = ksm_madvise(vma, start, end, behavior, &new_flags);
> - if (error)
> - goto out_convert_errno;
> - break;
> - case MADV_HUGEPAGE:
> - case MADV_NOHUGEPAGE:
> - error = hugepage_madvise(vma, &new_flags, behavior);
> - if (error)
> - goto out_convert_errno;
> - break;
> - }
>
> if (new_flags == vma->vm_flags) {
> *prev = vma;
> - goto out;
> + return 0;
> }
>
> pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT);
> @@ -149,21 +93,21 @@ static long madvise_behavior(struct vm_area_struct *vma,
> if (start != vma->vm_start) {
> if (unlikely(mm->map_count >= sysctl_max_map_count)) {
> error = -ENOMEM;
> - goto out;
> + return error;
Oh, I missed this one. Should be simply:
- error = -ENOMEM;
- goto out;
+ return -ENOMEM;
> }
> error = __split_vma(mm, vma, start, 1);
> if (error)
> - goto out_convert_errno;
> + return error;
> }
>
> if (end != vma->vm_end) {
> if (unlikely(mm->map_count >= sysctl_max_map_count)) {
> error = -ENOMEM;
> - goto out;
> + return error;
same here.
> }
> error = __split_vma(mm, vma, end, 0);
> if (error)
> - goto out_convert_errno;
> + return error;
> }
>
> success:
> @@ -172,15 +116,7 @@ static long madvise_behavior(struct vm_area_struct *vma,
> */
> vma->vm_flags = new_flags;
>
> -out_convert_errno:
> - /*
> - * madvise() returns EAGAIN if kernel resources, such as
> - * slab, are temporarily unavailable.
> - */
> - if (error == -ENOMEM)
> - error = -EAGAIN;
> -out:
> - return error;
> + return 0;
> }
>
> #ifdef CONFIG_SWAP
> @@ -930,6 +866,94 @@ static long madvise_remove(struct vm_area_struct *vma,
> return error;
> }
>
> +/*
> + * Apply an madvise behavior to a region of a vma. madvise_update_vma
> + * will handle splitting a vm area into separate areas, each area with its own
> + * behavior.
> + */
> +static int madvise_vma_behavior(struct vm_area_struct *vma,
> + struct vm_area_struct **prev,
> + unsigned long start, unsigned long end,
> + unsigned long behavior)
> +{
> + int error;
> + unsigned long new_flags = vma->vm_flags;
> +
> + switch (behavior) {
> + case MADV_REMOVE:
> + return madvise_remove(vma, prev, start, end);
> + case MADV_WILLNEED:
> + return madvise_willneed(vma, prev, start, end);
> + case MADV_COLD:
> + return madvise_cold(vma, prev, start, end);
> + case MADV_PAGEOUT:
> + return madvise_pageout(vma, prev, start, end);
> + case MADV_FREE:
> + case MADV_DONTNEED:
> + return madvise_dontneed_free(vma, prev, start, end, behavior);
> + case MADV_POPULATE_READ:
> + case MADV_POPULATE_WRITE:
> + return madvise_populate(vma, prev, start, end, behavior);
> + case MADV_NORMAL:
> + new_flags = new_flags & ~VM_RAND_READ & ~VM_SEQ_READ;
> + break;
> + case MADV_SEQUENTIAL:
> + new_flags = (new_flags & ~VM_RAND_READ) | VM_SEQ_READ;
> + break;
> + case MADV_RANDOM:
> + new_flags = (new_flags & ~VM_SEQ_READ) | VM_RAND_READ;
> + break;
> + case MADV_DONTFORK:
> + new_flags |= VM_DONTCOPY;
> + break;
> + case MADV_DOFORK:
> + if (vma->vm_flags & VM_IO)
> + return -EINVAL;
> + new_flags &= ~VM_DONTCOPY;
> + break;
> + case MADV_WIPEONFORK:
> + /* MADV_WIPEONFORK is only supported on anonymous memory. */
> + if (vma->vm_file || vma->vm_flags & VM_SHARED)
> + return -EINVAL;
> + new_flags |= VM_WIPEONFORK;
> + break;
> + case MADV_KEEPONFORK:
> + new_flags &= ~VM_WIPEONFORK;
> + break;
> + case MADV_DONTDUMP:
> + new_flags |= VM_DONTDUMP;
> + break;
> + case MADV_DODUMP:
> + if (!is_vm_hugetlb_page(vma) && new_flags & VM_SPECIAL)
> + return -EINVAL;
> + new_flags &= ~VM_DONTDUMP;
> + break;
> + case MADV_MERGEABLE:
> + case MADV_UNMERGEABLE:
> + error = ksm_madvise(vma, start, end, behavior, &new_flags);
> + if (error)
> + goto out;
> + break;
> + case MADV_HUGEPAGE:
> + case MADV_NOHUGEPAGE:
> + error = hugepage_madvise(vma, &new_flags, behavior);
> + if (error)
> + goto out;
> + break;
> + }
> +
> + error = madvise_update_vma(vma, prev, start, end, new_flags);
> +
> +out:
> + /*
> + * madvise() returns EAGAIN if kernel resources, such as
> + * slab, are temporarily unavailable.
> + */
> + if (error == -ENOMEM)
> + error = -EAGAIN;
> + return error;
> +}
> +
> #ifdef CONFIG_MEMORY_FAILURE
> /*
> * Error injection support for memory error handling.
> @@ -978,30 +1002,6 @@ static int madvise_inject_error(int behavior,
> }
> #endif
>
> -static long
> -madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
> - unsigned long start, unsigned long end, int behavior)
> -{
> - switch (behavior) {
> - case MADV_REMOVE:
> - return madvise_remove(vma, prev, start, end);
> - case MADV_WILLNEED:
> - return madvise_willneed(vma, prev, start, end);
> - case MADV_COLD:
> - return madvise_cold(vma, prev, start, end);
> - case MADV_PAGEOUT:
> - return madvise_pageout(vma, prev, start, end);
> - case MADV_FREE:
> - case MADV_DONTNEED:
> - return madvise_dontneed_free(vma, prev, start, end, behavior);
> - case MADV_POPULATE_READ:
> - case MADV_POPULATE_WRITE:
> - return madvise_populate(vma, prev, start, end, behavior);
> - default:
> - return madvise_behavior(vma, prev, start, end, behavior);
> - }
> -}
> -
> static bool
> madvise_behavior_valid(int behavior)
> {
> @@ -1054,6 +1054,73 @@ process_madvise_behavior_valid(int behavior)
> }
> }
>
> +/*
> + * Walk the vmas in range [start,end), and call the visit function on each one.
> + * The visit function will get start and end parameters that cover the overlap
> + * between the current vma and the original range. Any unmapped regions in the
> + * original range will result in this function returning -ENOMEM while still
> + * calling the visit function on all of the existing vmas in the range.
> + * Must be called with the mmap_lock held for reading or writing.
> + */
> +static
> +int madvise_walk_vmas(struct mm_struct *mm, unsigned long start,
> + unsigned long end, unsigned long arg,
> + int (*visit)(struct vm_area_struct *vma,
> + struct vm_area_struct **prev, unsigned long start,
> + unsigned long end, unsigned long arg))
> +{
> + struct vm_area_struct *vma;
> + struct vm_area_struct *prev;
> + unsigned long tmp;
> + int unmapped_error = 0;
> +
> + /*
> + * If the interval [start,end) covers some unmapped address
> + * ranges, just ignore them, but return -ENOMEM at the end.
> + * - different from the way of handling in mlock etc.
> + */
> + vma = find_vma_prev(mm, start, &prev);
> + if (vma && start > vma->vm_start)
> + prev = vma;
> +
> + for (;;) {
> + int error;
> +
> + /* Still start < end. */
> + if (!vma)
> + return -ENOMEM;
> +
> + /* Here start < (end|vma->vm_end). */
> + if (start < vma->vm_start) {
> + unmapped_error = -ENOMEM;
> + start = vma->vm_start;
> + if (start >= end)
> + break;
> + }
> +
> + /* Here vma->vm_start <= start < (end|vma->vm_end) */
> + tmp = vma->vm_end;
> + if (end < tmp)
> + tmp = end;
> +
> + /* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */
> + error = visit(vma, &prev, start, tmp, arg);
> + if (error)
> + return error;
> + start = tmp;
> + if (prev && start < prev->vm_end)
> + start = prev->vm_end;
> + if (start >= end)
> + break;
> + if (prev)
> + vma = prev->vm_next;
> + else /* madvise_remove dropped mmap_lock */
> + vma = find_vma(mm, start);
> + }
> +
> + return unmapped_error;
> +}
> +
> /*
> * The madvise(2) system call.
> *
> @@ -1126,9 +1193,7 @@ process_madvise_behavior_valid(int behavior)
> */
> int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int behavior)
> {
> - unsigned long end, tmp;
> - struct vm_area_struct *vma, *prev;
> - int unmapped_error = 0;
> + unsigned long end;
> int error = -EINVAL;
> int write;
> size_t len;
> @@ -1168,51 +1233,9 @@ int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int beh
> mmap_read_lock(mm);
> }
>
> - /*
> - * If the interval [start,end) covers some unmapped address
> - * ranges, just ignore them, but return -ENOMEM at the end.
> - * - different from the way of handling in mlock etc.
> - */
> - vma = find_vma_prev(mm, start, &prev);
> - if (vma && start > vma->vm_start)
> - prev = vma;
> -
> blk_start_plug(&plug);
> - for (;;) {
> - /* Still start < end. */
> - error = -ENOMEM;
> - if (!vma)
> - goto out;
> -
> - /* Here start < (end|vma->vm_end). */
> - if (start < vma->vm_start) {
> - unmapped_error = -ENOMEM;
> - start = vma->vm_start;
> - if (start >= end)
> - goto out;
> - }
> -
> - /* Here vma->vm_start <= start < (end|vma->vm_end) */
> - tmp = vma->vm_end;
> - if (end < tmp)
> - tmp = end;
> -
> - /* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */
> - error = madvise_vma(vma, &prev, start, tmp, behavior);
> - if (error)
> - goto out;
> - start = tmp;
> - if (prev && start < prev->vm_end)
> - start = prev->vm_end;
> - error = unmapped_error;
> - if (start >= end)
> - goto out;
> - if (prev)
> - vma = prev->vm_next;
> - else /* madvise_remove dropped mmap_lock */
> - vma = find_vma(mm, start);
> - }
> -out:
> + error = madvise_walk_vmas(mm, start, end, behavior,
> + madvise_vma_behavior);
> blk_finish_plug(&plug);
> if (write)
> mmap_write_unlock(mm);
> --
> 2.33.0.153.gba50c8fa24-goog
>
Powered by blists - more mailing lists