DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 081FA883AC
From: Joe Lawrence <joe.lawrence@redhat.com>
Organization: Red Hat
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Questions about commit "ipc/shm: Fix shmat mmap nil-page protection"
Cc: Davidlohr Bueso <dave@stgolabs.net>,
        Andrea Arcangeli <aarcange@redhat.com>
Message-ID: <472dbcaa-47b5-7a1b-7c4a-49373db784d3@redhat.com>
Date: Mon, 25 Sep 2017 15:38:07 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1593
Lines: 43

Hi Davidlohr,

I was looking into backporting commit 95e91b831f87 ("ipc/shm: Fix shmat
mmap nil-page protection") to a distro kernel and Andrea brought up some
interesting questions about that change.

We saw that a LTP test [1] was added some time ago to reproduce behavior
matching that of the original report [2].  However, Andrea and I are a
little confused about that original report and what the upstream commit
was intended to fix.  A quick summary of our offlist discussion:

- This is only about privileged users (and no SELinux).

- We modified the 20170119_shmat_nullpage_poc.c reproducer from [2] to
  include MAP_FIXED to prove (as root, no SELinux):

    It is possible to mmap 0
    It is NOT possible to mmap 1

- Andrea points out that mmap(1, ...) fails not because of any
  mmap_min_addr checks, but for alignment reasons.

- He also wonders about other bogus addr values above 4k, but below
  mmap_min_addr and whether this change misses those values


Is it possible that the original report noticed that shmat allowed
attach to an address of 1, and it was assumed that somehow mmap_min_addr
protections were circumvented?  Then commit 95e91b831f87 modified the
rounding in do_shmat() so that shmat would fail on similar input (but
for apparently different reasons)?

I didn't see any discussion when looking up the original commit in the
list archives, so any explanations or pointers would be very helpful.

[1]
https://github.com/linux-test-project/ltp/blob/master/testcases/cve/cve-2017-5669.c

[2] https://bugzilla.kernel.org/show_bug.cgi?id=192931

Regards,

-- Joe