linux-kernel - Re: [PATCH] mm: check VMA flags to avoid invalid PROT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161007100720.GA14859@lucifer>
Date:   Fri, 7 Oct 2016 11:07:20 +0100
From:   Lorenzo Stoakes <lstoakes@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Rik van Riel <riel@...hat.com>, Hugh Dickins <hughd@...gle.com>,
        linux-mm <linux-mm@...ck.org>,
        Mel Gorman <mgorman@...hsingularity.net>,
        tbsaunde@...aunde.org, robert@...llahan.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] mm: check VMA flags to avoid invalid PROT_NONE NUMA
 balancing

On Sun, Sep 25, 2016 at 03:50:21PM -0700, Linus Torvalds wrote:
> I'd really like to re-open the "drop FOLL_FORCE entirely" discussion,
> because the thing really is disgusting.
>
> I realize that debuggers etc sometimes would want to punch through
> PROT_NONE protections, and I also realize that right now we only have
> a read/write flag, and we have that whole issue with "what if it's
> executable but not readable", which currently FOLL_FORCE makes a
> non-issue.

So I've experimented with this a little locally, removing FOLL_FORCE altogether
and tracking places where it is used (it seems to be a fair few places
actually.)

I've rather naively replaced the FOLL_FORCE check in check_vma_flags() with a
check against 'tsk && tsk->ptrace && tsk->parent == current', I'm not sure how
valid or sane this is, however, but a quick check against gdb proves that it is
able to do its thing in this configuration. Is this a viable path, or is this
way off the mark here?

The places I've found that have invoked gup functions which eventually result in
FOLL_FORCE being set are:

Calls __get_user_pages():
	mm/gup.c: populate_vma_page_range()
	mm/gup.c: get_dump_page()

calls get_user_pages_unlocked():
	drivers/media/pci/ivtv/ivtv-yuv.c: ivtv_yuv_prep_user_dma()
	drivers/media/pci/ivtv/ivtv-udma.c: ivtv_udma_setup()

calls get_user_pages_remote():
	mm/memory.c: __access_remote_vm() [ see below for callers ]
	fs/exec.c: get_arg_page()
	kernel/events/uprobes.c: uprobe_write_opcode()
	kernel/events/uprobes.c: is_trap_at_addr()
	security/tomoyo/domain.c: tomoyo_dump_page()

calls __access_remote_vm():
	mm/memory.c: access_remote_vm() [ see below for callers ]
	mm/memory.c: access_process_vm()

access_process_vm() is exclusively used for ptrace, omitting its callers here.

calls access_remote_vm():
	fs/proc/base.c: proc_pid_cmdline_read()
	fs/proc/base.c: memrw()
	fs/proc/base.c: environ_read()

calls get_user_pages():
	drivers/infiniband/core/umem.c: ib_umem_get()
	drivers/infiniband/hw/qib/qib_user_pages.c: __qib_get_user_pages()
	drivers/infiniband/hw/usnic/usnic_uiom.c: usnic_uiom_get_pages()
	drivers/media/v4l2-core/videobuf-dma-sg.c: videobuf_dma_init_user_locked()

calls get_vaddr_frames():
	drivers/media/v4l2-core/videobuf2-memops.c: vb2_create_framevec()
	drivers/gpu/drm/exynos/exynos_drm_g2d.c: g2d_userptr_get_dma_addr()

So it seems the general areas where it is used are tracing, uprobes and DMA
initialisation what what I can tell. I'm thinking some extra provision/careful
checking will be needed in each of these cases to see if an alternative is
possible.

I'm happy to explore this some more if that is useful in any way, though of
course I defer to your expertise as to how a world without FOLL_FORCE might
look!

Cheers, Lorenzo