| Message ID | 20180921150351.20898-25-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control Flow Enforcement: Shadow Stack | expand |
On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote:
> Create a guard area between VMAs, to detect memory corruption.
Do I understand correctly that with this patch a user space program
no longer be able to place two mappings back to back? If it is so,
it will likely break a lot of things; for example, it's a common ring
buffer implementations technique, to map buffer memory twice back
to back in order to avoid special handling of items wrapping its end.
On Tue, Oct 2, 2018 at 9:55 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > > On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote: > > Create a guard area between VMAs, to detect memory corruption. > > Do I understand correctly that with this patch a user space program > no longer be able to place two mappings back to back? If it is so, > it will likely break a lot of things; for example, it's a common ring > buffer implementations technique, to map buffer memory twice back > to back in order to avoid special handling of items wrapping its end. I haven't checked what the patch actually does, but it shouldn't have any affect on MAP_FIXED or the new no-replace MAP_FIXED variant. --Andy
On Tue, 2018-10-02 at 22:36 -0700, Andy Lutomirski wrote: > On Tue, Oct 2, 2018 at 9:55 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > > > > On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote: > > > Create a guard area between VMAs, to detect memory corruption. > > > > Do I understand correctly that with this patch a user space program > > no longer be able to place two mappings back to back? If it is so, > > it will likely break a lot of things; for example, it's a common ring > > buffer implementations technique, to map buffer memory twice back > > to back in order to avoid special handling of items wrapping its end. > > I haven't checked what the patch actually does, but it shouldn't have > any affect on MAP_FIXED or the new no-replace MAP_FIXED variant. > > --Andy I did some mmap tests with/without MAP_FIXED, and it works as intended. In addition to the ring buffer, are there other test cases? Yu-cheng
On Wed, Oct 3, 2018 at 9:06 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: > > On Tue, 2018-10-02 at 22:36 -0700, Andy Lutomirski wrote: > > On Tue, Oct 2, 2018 at 9:55 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > > > > > > On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote: > > > > Create a guard area between VMAs, to detect memory corruption. > > > > > > Do I understand correctly that with this patch a user space program > > > no longer be able to place two mappings back to back? If it is so, > > > it will likely break a lot of things; for example, it's a common ring > > > buffer implementations technique, to map buffer memory twice back > > > to back in order to avoid special handling of items wrapping its end. > > > > I haven't checked what the patch actually does, but it shouldn't have > > any affect on MAP_FIXED or the new no-replace MAP_FIXED variant. > > > > --Andy > > I did some mmap tests with/without MAP_FIXED, and it works as intended. > In addition to the ring buffer, are there other test cases? > Various ELF loaders, perhaps? Do they use MAP_FIXED or do they just use address hints?
On Wed, Oct 03, 2018 at 09:00:04AM -0700, Yu-cheng Yu wrote: > On Tue, 2018-10-02 at 22:36 -0700, Andy Lutomirski wrote: > > On Tue, Oct 2, 2018 at 9:55 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > > > > > > On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote: > > > > Create a guard area between VMAs, to detect memory corruption. > > > > > > Do I understand correctly that with this patch a user space program > > > no longer be able to place two mappings back to back? If it is so, > > > it will likely break a lot of things; for example, it's a common ring > > > buffer implementations technique, to map buffer memory twice back > > > to back in order to avoid special handling of items wrapping its end. > > > > I haven't checked what the patch actually does, but it shouldn't have > > any affect on MAP_FIXED or the new no-replace MAP_FIXED variant. > > > > --Andy > > I did some mmap tests with/without MAP_FIXED, and it works as intended. > In addition to the ring buffer, are there other test cases? Right, after some more code reading I figured out that it indeed shouldn't affect MAP_FIXED, thank you for confirmation. I'm not sure, however, whether such a change that provides no ability to configure or affect it will go well with all the supported architectures.
On Wed, 2018-10-03 at 18:32 +0200, Eugene Syromiatnikov wrote: > On Wed, Oct 03, 2018 at 09:00:04AM -0700, Yu-cheng Yu wrote: > > On Tue, 2018-10-02 at 22:36 -0700, Andy Lutomirski wrote: > > > On Tue, Oct 2, 2018 at 9:55 PM Eugene Syromiatnikov <esyr@redhat.com> > > > wrote: > > > > > > > > On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote: > > > > > Create a guard area between VMAs, to detect memory corruption. > > > > > > > > Do I understand correctly that with this patch a user space program > > > > no longer be able to place two mappings back to back? If it is so, > > > > it will likely break a lot of things; for example, it's a common ring > > > > buffer implementations technique, to map buffer memory twice back > > > > to back in order to avoid special handling of items wrapping its end. > > > > > > I haven't checked what the patch actually does, but it shouldn't have > > > any affect on MAP_FIXED or the new no-replace MAP_FIXED variant. > > > > > > --Andy > > > > I did some mmap tests with/without MAP_FIXED, and it works as intended. > > In addition to the ring buffer, are there other test cases? > > Right, after some more code reading I figured out that it indeed > shouldn't affect MAP_FIXED, thank you for confirmation. > > I'm not sure, however, whether such a change that provides no ability > to configure or affect it will go well with all the supported > architectures. Can we do CONFIG_MMAP_GUARD_GAP?
On Wed, Oct 3, 2018 at 6:32 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > On Wed, Oct 03, 2018 at 09:00:04AM -0700, Yu-cheng Yu wrote: > > On Tue, 2018-10-02 at 22:36 -0700, Andy Lutomirski wrote: > > > On Tue, Oct 2, 2018 at 9:55 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > > > > > > > > On Fri, Sep 21, 2018 at 08:03:48AM -0700, Yu-cheng Yu wrote: > > > > > Create a guard area between VMAs, to detect memory corruption. > > > > > > > > Do I understand correctly that with this patch a user space program > > > > no longer be able to place two mappings back to back? If it is so, > > > > it will likely break a lot of things; for example, it's a common ring > > > > buffer implementations technique, to map buffer memory twice back > > > > to back in order to avoid special handling of items wrapping its end. > > > > > > I haven't checked what the patch actually does, but it shouldn't have > > > any affect on MAP_FIXED or the new no-replace MAP_FIXED variant. > > > > > > --Andy > > > > I did some mmap tests with/without MAP_FIXED, and it works as intended. > > In addition to the ring buffer, are there other test cases? > > Right, after some more code reading I figured out that it indeed > shouldn't affect MAP_FIXED, thank you for confirmation. > > I'm not sure, however, whether such a change that provides no ability > to configure or affect it will go well with all the supported > architectures. Is there a concrete reason why you think an architecture might not like this? As far as I can tell, the virtual address space overhead should be insignificant even for 32-bit systems.
On Wed, Oct 03, 2018 at 06:52:40PM +0200, Jann Horn wrote: > On Wed, Oct 3, 2018 at 6:32 PM Eugene Syromiatnikov <esyr@redhat.com> wrote: > > I'm not sure, however, whether such a change that provides no ability > > to configure or affect it will go well with all the supported > > architectures. > > Is there a concrete reason why you think an architecture might not > like this? As far as I can tell, the virtual address space overhead > should be insignificant even for 32-bit systems. Not really, and not architectures per se, but judging by some past experiences with enabling ASLR, I would expect that all kinds of weird applications may start to behave in all kinds of strange ways. Not that I have anything more than this doubt, however; but this sort of change without any ability to tune or revert it still looks unusual to me.
diff --git a/include/linux/mm.h b/include/linux/mm.h index c4cc07baccda..3a823bdae09d 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2443,24 +2443,34 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m static inline unsigned long vm_start_gap(struct vm_area_struct *vma) { unsigned long vm_start = vma->vm_start; + unsigned long gap; + + if (vma->vm_flags & VM_GROWSDOWN) + gap = stack_guard_gap; + else + gap = PAGE_SIZE; + + vm_start -= gap; + if (vm_start > vma->vm_start) + vm_start = 0; - if (vma->vm_flags & VM_GROWSDOWN) { - vm_start -= stack_guard_gap; - if (vm_start > vma->vm_start) - vm_start = 0; - } return vm_start; } static inline unsigned long vm_end_gap(struct vm_area_struct *vma) { unsigned long vm_end = vma->vm_end; + unsigned long gap; + + if (vma->vm_flags & VM_GROWSUP) + gap = stack_guard_gap; + else + gap = PAGE_SIZE; + + vm_end += gap; + if (vm_end < vma->vm_end) + vm_end = -PAGE_SIZE; - if (vma->vm_flags & VM_GROWSUP) { - vm_end += stack_guard_gap; - if (vm_end < vma->vm_end) - vm_end = -PAGE_SIZE; - } return vm_end; }
Create a guard area between VMAs, to detect memory corruption. Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> --- include/linux/mm.h | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)