RE: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
> -Original Message- > From: Naresh Kamboju [mailto:naresh.kamb...@linaro.org] > Sent: Friday, June 1, 2018 12:55 AM > To: Daniel Sangorrin > Cc: Greg Kroah-Hartman ; open list > ; linux- stable ; > Davidlohr Bueso ; Joe Lawrence ; > Andrea Arcangeli ; Manfred Spraul > ; Andrew Morton ; > Linus Torvalds > Subject: Re: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page > protection" > > On 31 May 2018 at 13:06, Daniel Sangorrin > wrote: > >> -Original Message- > >> From: 'Greg Kroah-Hartman' [mailto:gre...@linuxfoundation.org] > > .. > >> Thanks for letting us know, but this was reported already. See the > >> emails on lkml with the subject: > >> Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review > >> from Davidlohr Bueso > >> Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> > >> > >> where he discusses that the LTP test is incorrect and that the kernel > >> change is correct and that LTP is going to be fixed because of this. > > My two cents, > If you are referring to cve-2017-5669.c > LTP test case is been fixed few hours ago by Rafael Tinoco, > > - shm_addr = shmat(shm_id, ((void *)1), SHM_RND); > + shm_addr = shmat(shm_id, ((void *)1), SHM_RND | SHM_REMAP); > > LTP patch pull request and it is been merged. > https://github.com/linux-test-project/ltp/pull/324 Thanks a lot Naresh. I confirmed that the latest LTP cve-2017-5669 now PASSes. Thanks, Daniel
RE: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
> -Original Message- > From: Naresh Kamboju [mailto:naresh.kamb...@linaro.org] > Sent: Friday, June 1, 2018 12:55 AM > To: Daniel Sangorrin > Cc: Greg Kroah-Hartman ; open list > ; linux- stable ; > Davidlohr Bueso ; Joe Lawrence ; > Andrea Arcangeli ; Manfred Spraul > ; Andrew Morton ; > Linus Torvalds > Subject: Re: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page > protection" > > On 31 May 2018 at 13:06, Daniel Sangorrin > wrote: > >> -Original Message- > >> From: 'Greg Kroah-Hartman' [mailto:gre...@linuxfoundation.org] > > .. > >> Thanks for letting us know, but this was reported already. See the > >> emails on lkml with the subject: > >> Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review > >> from Davidlohr Bueso > >> Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> > >> > >> where he discusses that the LTP test is incorrect and that the kernel > >> change is correct and that LTP is going to be fixed because of this. > > My two cents, > If you are referring to cve-2017-5669.c > LTP test case is been fixed few hours ago by Rafael Tinoco, > > - shm_addr = shmat(shm_id, ((void *)1), SHM_RND); > + shm_addr = shmat(shm_id, ((void *)1), SHM_RND | SHM_REMAP); > > LTP patch pull request and it is been merged. > https://github.com/linux-test-project/ltp/pull/324 Thanks a lot Naresh. I confirmed that the latest LTP cve-2017-5669 now PASSes. Thanks, Daniel
Re: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
On 31 May 2018 at 13:06, Daniel Sangorrin wrote: >> -Original Message- >> From: 'Greg Kroah-Hartman' [mailto:gre...@linuxfoundation.org] > .. >> Thanks for letting us know, but this was reported already. See the >> emails on lkml with the subject: >> Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review >> from Davidlohr Bueso >> Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> >> >> where he discusses that the LTP test is incorrect and that the kernel >> change is correct and that LTP is going to be fixed because of this. My two cents, If you are referring to cve-2017-5669.c LTP test case is been fixed few hours ago by Rafael Tinoco, - shm_addr = shmat(shm_id, ((void *)1), SHM_RND); + shm_addr = shmat(shm_id, ((void *)1), SHM_RND | SHM_REMAP); LTP patch pull request and it is been merged. https://github.com/linux-test-project/ltp/pull/324 > > OK, thank you for the information! > - Naresh > Regards, > Daniel > >
Re: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
On 31 May 2018 at 13:06, Daniel Sangorrin wrote: >> -Original Message- >> From: 'Greg Kroah-Hartman' [mailto:gre...@linuxfoundation.org] > .. >> Thanks for letting us know, but this was reported already. See the >> emails on lkml with the subject: >> Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review >> from Davidlohr Bueso >> Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> >> >> where he discusses that the LTP test is incorrect and that the kernel >> change is correct and that LTP is going to be fixed because of this. My two cents, If you are referring to cve-2017-5669.c LTP test case is been fixed few hours ago by Rafael Tinoco, - shm_addr = shmat(shm_id, ((void *)1), SHM_RND); + shm_addr = shmat(shm_id, ((void *)1), SHM_RND | SHM_REMAP); LTP patch pull request and it is been merged. https://github.com/linux-test-project/ltp/pull/324 > > OK, thank you for the information! > - Naresh > Regards, > Daniel > >
RE: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
> -Original Message- > From: 'Greg Kroah-Hartman' [mailto:gre...@linuxfoundation.org] .. > Thanks for letting us know, but this was reported already. See the > emails on lkml with the subject: > Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review > from Davidlohr Bueso > Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> > > where he discusses that the LTP test is incorrect and that the kernel > change is correct and that LTP is going to be fixed because of this. OK, thank you for the information! Regards, Daniel
RE: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
> -Original Message- > From: 'Greg Kroah-Hartman' [mailto:gre...@linuxfoundation.org] .. > Thanks for letting us know, but this was reported already. See the > emails on lkml with the subject: > Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review > from Davidlohr Bueso > Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> > > where he discusses that the LTP test is incorrect and that the kernel > change is correct and that LTP is going to be fixed because of this. OK, thank you for the information! Regards, Daniel
Re: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
On Thu, May 31, 2018 at 11:36:46AM +0900, Daniel Sangorrin wrote: > > -Original Message- > > From: stable-ow...@vger.kernel.org [mailto:stable-ow...@vger.kernel.org] On > > 4.4-stable review patch. If anyone has any objections, please let me know. > > > > -- > > > > From: Davidlohr Bueso > > > > commit a73ab244f0dad8fffb3291b905f73e2d3eaa7c00 upstream. > > > > Patch series "ipc/shm: shmat() fixes around nil-page". > > Sorry for being a bit late (the pace is really fast here). > > I have found a regression from 4.4.133-rc1 to 4.4.134-rc1 using Fuego LTP > wrapper. > > 4.4.134-rc1 > tst_test.c:982: INFO: Timeout per run is 0h 05m 00s > cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null > page > cve-2017-5669.c:74: INFO: Mapped shared memory to (nil) > cve-2017-5669.c:78: FAIL: We have mapped a VM address within the first > 64Kb > cve-2017-5669.c:84: INFO: Touching shared memory to see if anything > strange happens > > 4.4.133-rc1: > tst_test.c:982: INFO: Timeout per run is 0h 05m 00s > cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null > page > cve-2017-5669.c:67: PASS: shmat returned EINVAL > > The culprits should be one or both of the two last commits to ipc/shm (one of > them a revert). > > - ipc/shm: fix shmat() nil address after round-down when remapping > - Revert "ipc/shm: Fix shmat mmap nil-page protection" > > I need to investigate the concrete reason, but for now I just wanted to > report it. Thanks for letting us know, but this was reported already. See the emails on lkml with the subject: Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review from Davidlohr Bueso Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> where he discusses that the LTP test is incorrect and that the kernel change is correct and that LTP is going to be fixed because of this. thanks, greg k-h
Re: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
On Thu, May 31, 2018 at 11:36:46AM +0900, Daniel Sangorrin wrote: > > -Original Message- > > From: stable-ow...@vger.kernel.org [mailto:stable-ow...@vger.kernel.org] On > > 4.4-stable review patch. If anyone has any objections, please let me know. > > > > -- > > > > From: Davidlohr Bueso > > > > commit a73ab244f0dad8fffb3291b905f73e2d3eaa7c00 upstream. > > > > Patch series "ipc/shm: shmat() fixes around nil-page". > > Sorry for being a bit late (the pace is really fast here). > > I have found a regression from 4.4.133-rc1 to 4.4.134-rc1 using Fuego LTP > wrapper. > > 4.4.134-rc1 > tst_test.c:982: INFO: Timeout per run is 0h 05m 00s > cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null > page > cve-2017-5669.c:74: INFO: Mapped shared memory to (nil) > cve-2017-5669.c:78: FAIL: We have mapped a VM address within the first > 64Kb > cve-2017-5669.c:84: INFO: Touching shared memory to see if anything > strange happens > > 4.4.133-rc1: > tst_test.c:982: INFO: Timeout per run is 0h 05m 00s > cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null > page > cve-2017-5669.c:67: PASS: shmat returned EINVAL > > The culprits should be one or both of the two last commits to ipc/shm (one of > them a revert). > > - ipc/shm: fix shmat() nil address after round-down when remapping > - Revert "ipc/shm: Fix shmat mmap nil-page protection" > > I need to investigate the concrete reason, but for now I just wanted to > report it. Thanks for letting us know, but this was reported already. See the emails on lkml with the subject: Subject: Re: [PATCH 4.16 000/272] 4.16.13-stable review from Davidlohr Bueso Message-ID: <20180528213039.yy2madue67njkmw5@linux-n805> where he discusses that the LTP test is incorrect and that the kernel change is correct and that LTP is going to be fixed because of this. thanks, greg k-h
RE: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
> -Original Message-
> From: stable-ow...@vger.kernel.org [mailto:stable-ow...@vger.kernel.org] On
> 4.4-stable review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Davidlohr Bueso
>
> commit a73ab244f0dad8fffb3291b905f73e2d3eaa7c00 upstream.
>
> Patch series "ipc/shm: shmat() fixes around nil-page".
Sorry for being a bit late (the pace is really fast here).
I have found a regression from 4.4.133-rc1 to 4.4.134-rc1 using Fuego LTP
wrapper.
4.4.134-rc1
tst_test.c:982: INFO: Timeout per run is 0h 05m 00s
cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null
page
cve-2017-5669.c:74: INFO: Mapped shared memory to (nil)
cve-2017-5669.c:78: FAIL: We have mapped a VM address within the first
64Kb
cve-2017-5669.c:84: INFO: Touching shared memory to see if anything
strange happens
4.4.133-rc1:
tst_test.c:982: INFO: Timeout per run is 0h 05m 00s
cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null
page
cve-2017-5669.c:67: PASS: shmat returned EINVAL
The culprits should be one or both of the two last commits to ipc/shm (one of
them a revert).
- ipc/shm: fix shmat() nil address after round-down when remapping
- Revert "ipc/shm: Fix shmat mmap nil-page protection"
I need to investigate the concrete reason, but for now I just wanted to report
it.
Thanks,
Daniel
>
> These patches fix two issues reported[1] a while back by Joe and Andrea
> around how shmat(2) behaves with nil-page.
>
> The first reverts a commit that it was incorrectly thought that mapping
> nil-page (address=0) was a no no with MAP_FIXED. This is not the case,
> with the exception of SHM_REMAP; which is address in the second patch.
>
> I chose two patches because it is easier to backport and it explicitly
> reverts bogus behaviour. Both patches ought to be in -stable and ltp
> testcases need updated (the added testcase around the cve can be
> modified to just test for SHM_RND|SHM_REMAP).
>
> [1] lkml.kernel.org/r/20180430172152.nfa564pvgpk3ut7p@linux-n805
>
> This patch (of 2):
>
> Commit 95e91b831f87 ("ipc/shm: Fix shmat mmap nil-page protection")
> worked on the idea that we should not be mapping as root addr=0 and
> MAP_FIXED. However, it was reported that this scenario is in fact
> valid, thus making the patch both bogus and breaks userspace as well.
>
> For example X11's libint10.so relies on shmat(1, SHM_RND) for lowmem
> initialization[1].
>
> [1]
> https://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/os-support/linux/int1
> 0/linux.c#n347
> Link: http://lkml.kernel.org/r/20180503203243.15045-2-d...@stgolabs.net
> Fixes: 95e91b831f87 ("ipc/shm: Fix shmat mmap nil-page protection")
> Signed-off-by: Davidlohr Bueso
> Reported-by: Joe Lawrence
> Reported-by: Andrea Arcangeli
> Cc: Manfred Spraul
> Cc:
> Signed-off-by: Andrew Morton
> Signed-off-by: Linus Torvalds
> Signed-off-by: Greg Kroah-Hartman
>
> ---
> ipc/shm.c |9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -1113,13 +1113,8 @@ long do_shmat(int shmid, char __user *sh
> goto out;
> else if ((addr = (ulong)shmaddr)) {
> if (addr & (shmlba - 1)) {
> - /*
> - * Round down to the nearest multiple of shmlba.
> - * For sane do_mmap_pgoff() parameters, avoid
> - * round downs that trigger nil-page and MAP_FIXED.
> - */
> - if ((shmflg & SHM_RND) && addr >= shmlba)
> - addr &= ~(shmlba - 1);
> + if (shmflg & SHM_RND)
> + addr &= ~(shmlba - 1); /* round down */
> else
> #ifndef __ARCH_FORCE_SHMLBA
> if (addr & ~PAGE_MASK)
>
>
RE: [PATCH 4.4 011/268] Revert "ipc/shm: Fix shmat mmap nil-page protection"
> -Original Message-
> From: stable-ow...@vger.kernel.org [mailto:stable-ow...@vger.kernel.org] On
> 4.4-stable review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Davidlohr Bueso
>
> commit a73ab244f0dad8fffb3291b905f73e2d3eaa7c00 upstream.
>
> Patch series "ipc/shm: shmat() fixes around nil-page".
Sorry for being a bit late (the pace is really fast here).
I have found a regression from 4.4.133-rc1 to 4.4.134-rc1 using Fuego LTP
wrapper.
4.4.134-rc1
tst_test.c:982: INFO: Timeout per run is 0h 05m 00s
cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null
page
cve-2017-5669.c:74: INFO: Mapped shared memory to (nil)
cve-2017-5669.c:78: FAIL: We have mapped a VM address within the first
64Kb
cve-2017-5669.c:84: INFO: Touching shared memory to see if anything
strange happens
4.4.133-rc1:
tst_test.c:982: INFO: Timeout per run is 0h 05m 00s
cve-2017-5669.c:62: INFO: Attempting to attach shared memory to null
page
cve-2017-5669.c:67: PASS: shmat returned EINVAL
The culprits should be one or both of the two last commits to ipc/shm (one of
them a revert).
- ipc/shm: fix shmat() nil address after round-down when remapping
- Revert "ipc/shm: Fix shmat mmap nil-page protection"
I need to investigate the concrete reason, but for now I just wanted to report
it.
Thanks,
Daniel
>
> These patches fix two issues reported[1] a while back by Joe and Andrea
> around how shmat(2) behaves with nil-page.
>
> The first reverts a commit that it was incorrectly thought that mapping
> nil-page (address=0) was a no no with MAP_FIXED. This is not the case,
> with the exception of SHM_REMAP; which is address in the second patch.
>
> I chose two patches because it is easier to backport and it explicitly
> reverts bogus behaviour. Both patches ought to be in -stable and ltp
> testcases need updated (the added testcase around the cve can be
> modified to just test for SHM_RND|SHM_REMAP).
>
> [1] lkml.kernel.org/r/20180430172152.nfa564pvgpk3ut7p@linux-n805
>
> This patch (of 2):
>
> Commit 95e91b831f87 ("ipc/shm: Fix shmat mmap nil-page protection")
> worked on the idea that we should not be mapping as root addr=0 and
> MAP_FIXED. However, it was reported that this scenario is in fact
> valid, thus making the patch both bogus and breaks userspace as well.
>
> For example X11's libint10.so relies on shmat(1, SHM_RND) for lowmem
> initialization[1].
>
> [1]
> https://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/os-support/linux/int1
> 0/linux.c#n347
> Link: http://lkml.kernel.org/r/20180503203243.15045-2-d...@stgolabs.net
> Fixes: 95e91b831f87 ("ipc/shm: Fix shmat mmap nil-page protection")
> Signed-off-by: Davidlohr Bueso
> Reported-by: Joe Lawrence
> Reported-by: Andrea Arcangeli
> Cc: Manfred Spraul
> Cc:
> Signed-off-by: Andrew Morton
> Signed-off-by: Linus Torvalds
> Signed-off-by: Greg Kroah-Hartman
>
> ---
> ipc/shm.c |9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -1113,13 +1113,8 @@ long do_shmat(int shmid, char __user *sh
> goto out;
> else if ((addr = (ulong)shmaddr)) {
> if (addr & (shmlba - 1)) {
> - /*
> - * Round down to the nearest multiple of shmlba.
> - * For sane do_mmap_pgoff() parameters, avoid
> - * round downs that trigger nil-page and MAP_FIXED.
> - */
> - if ((shmflg & SHM_RND) && addr >= shmlba)
> - addr &= ~(shmlba - 1);
> + if (shmflg & SHM_RND)
> + addr &= ~(shmlba - 1); /* round down */
> else
> #ifndef __ARCH_FORCE_SHMLBA
> if (addr & ~PAGE_MASK)
>
>
