2017-12-14 11:43:59

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH v2 02/17] mm: Exempt special mappings from mlock(), mprotect() and madvise()

It makes no sense to ever prod at special mappings with any of these
syscalls.

XXX should we include munmap() ?

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
mm/madvise.c | 3 +++
mm/mlock.c | 3 ++-
mm/mprotect.c | 3 +++
3 files changed, 8 insertions(+), 1 deletion(-)

--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -678,6 +678,9 @@ static long
madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
unsigned long start, unsigned long end, int behavior)
{
+ if (vma_is_special_mapping(vma))
+ return -EINVAL;
+
switch (behavior) {
case MADV_REMOVE:
return madvise_remove(vma, prev, start, end);
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -521,7 +521,8 @@ static int mlock_fixup(struct vm_area_st
vm_flags_t old_flags = vma->vm_flags;

if (newflags == vma->vm_flags || (vma->vm_flags & VM_SPECIAL) ||
- is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm))
+ is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm) ||
+ vma_is_special_mapping(vma))
/* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
goto out;

--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -307,6 +307,9 @@ mprotect_fixup(struct vm_area_struct *vm
return 0;
}

+ if (vma_is_special_mapping(vma))
+ return -ENOMEM;
+
/*
* If we make a private mapping writable we increase our commit;
* but (without finer accounting) cannot reduce our commit if we



2017-12-14 16:20:01

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v2 02/17] mm: Exempt special mappings from mlock(), mprotect() and madvise()

On Thu, Dec 14, 2017 at 3:27 AM, Peter Zijlstra <[email protected]> wrote:
> It makes no sense to ever prod at special mappings with any of these
> syscalls.
>
> XXX should we include munmap() ?

This is an ABI break for the vdso. Maybe that's okay, but mremap() on
the vdso is certainly used, and I can imagine debuggers using
mprotect().

2017-12-14 17:37:17

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 02/17] mm: Exempt special mappings from mlock(), mprotect() and madvise()

On Thu, Dec 14, 2017 at 08:19:36AM -0800, Andy Lutomirski wrote:
> On Thu, Dec 14, 2017 at 3:27 AM, Peter Zijlstra <[email protected]> wrote:
> > It makes no sense to ever prod at special mappings with any of these
> > syscalls.
> >
> > XXX should we include munmap() ?
>
> This is an ABI break for the vdso. Maybe that's okay, but mremap() on
> the vdso is certainly used, and I can imagine debuggers using
> mprotect().

*groan*, ok so mremap() will actually still work after this, but yes,
mprotect() will not. I hadn't figured people would muck with the VDSO
like that.

2018-01-02 16:44:46

by Dmitry Safonov

[permalink] [raw]
Subject: Re: [PATCH v2 02/17] mm: Exempt special mappings from mlock(), mprotect() and madvise()

Hi, sorry for the late reply,

2017-12-14 17:36 GMT+00:00 Peter Zijlstra <[email protected]>:
> On Thu, Dec 14, 2017 at 08:19:36AM -0800, Andy Lutomirski wrote:
>> On Thu, Dec 14, 2017 at 3:27 AM, Peter Zijlstra <[email protected]> wrote:
>> > It makes no sense to ever prod at special mappings with any of these
>> > syscalls.
>> >
>> > XXX should we include munmap() ?
>>
>> This is an ABI break for the vdso. Maybe that's okay, but mremap() on
>> the vdso is certainly used, and I can imagine debuggers using
>> mprotect().
>
> *groan*, ok so mremap() will actually still work after this, but yes,
> mprotect() will not. I hadn't figured people would muck with the VDSO
> like that.

mremap() is needed for CRIU, at least.

Please, don't restrict munmap(), as ARCH_MAP_VDSO_* allows to map vdso
iff it's not already mapped.
We don't need +w vdso mapping, but I guess that may break gdb breakpoints
on vdso.

Also, AFAICS, vma_is_special_mapping() has two parameters in linux-next,
and your patches set doesn't change that.

Thanks,
Dmitry