* [Qemu-devel] [PULL 1/3] linux-user/mmap.c: handle invalid len maps correctly
2018-07-31 8:42 [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches Laurent Vivier
@ 2018-07-31 8:42 ` Laurent Vivier
2018-07-31 8:42 ` [Qemu-devel] [PULL 2/3] tests: add check_invalid_maps to test-mmap Laurent Vivier
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-07-31 8:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Alex Bennée, Laurent Vivier, umarcor
From: Alex Bennée <alex.bennee@linaro.org>
I've slightly re-organised the check to more closely match the
sequence that the kernel uses in do_mmap(). We check for both the zero
case (EINVAL) and the overflow length case (ENOMEM).
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: umarcor <1783362@bugs.launchpad.net>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20180730134321.19898-2-alex.bennee@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/mmap.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index d0c50e4888..41e0983ce8 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -391,14 +391,23 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
}
#endif
- if (offset & ~TARGET_PAGE_MASK) {
+ if (!len) {
errno = EINVAL;
goto fail;
}
+ /* Also check for overflows... */
len = TARGET_PAGE_ALIGN(len);
- if (len == 0)
- goto the_end;
+ if (!len) {
+ errno = ENOMEM;
+ goto fail;
+ }
+
+ if (offset & ~TARGET_PAGE_MASK) {
+ errno = EINVAL;
+ goto fail;
+ }
+
real_start = start & qemu_host_page_mask;
host_offset = offset & qemu_host_page_mask;
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 2/3] tests: add check_invalid_maps to test-mmap
2018-07-31 8:42 [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches Laurent Vivier
2018-07-31 8:42 ` [Qemu-devel] [PULL 1/3] linux-user/mmap.c: handle invalid len maps correctly Laurent Vivier
@ 2018-07-31 8:42 ` Laurent Vivier
2018-07-31 8:42 ` [Qemu-devel] [PULL 3/3] linux-user: ppc64: don't use volatile register during safe_syscall Laurent Vivier
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-07-31 8:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Alex Bennée, Laurent Vivier, umarcor
From: Alex Bennée <alex.bennee@linaro.org>
This adds a test to make sure we fail properly for a 0 length mmap.
There are most likely other failure conditions we should also check.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Cc: umarcor <1783362@bugs.launchpad.net>
Message-Id: <20180730134321.19898-3-alex.bennee@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
tests/tcg/multiarch/test-mmap.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
index 5c0afe6e49..11d0e777b1 100644
--- a/tests/tcg/multiarch/test-mmap.c
+++ b/tests/tcg/multiarch/test-mmap.c
@@ -27,7 +27,7 @@
#include <stdint.h>
#include <string.h>
#include <unistd.h>
-
+#include <errno.h>
#include <sys/mman.h>
#define D(x)
@@ -435,6 +435,25 @@ void checked_write(int fd, const void *buf, size_t count)
fail_unless(rc == count);
}
+void check_invalid_mmaps(void)
+{
+ unsigned char *addr;
+
+ /* Attempt to map a zero length page. */
+ addr = mmap(NULL, 0, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
+ fail_unless(addr == MAP_FAILED);
+ fail_unless(errno == EINVAL);
+
+ /* Attempt to map a over length page. */
+ addr = mmap(NULL, -4, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
+ fail_unless(addr == MAP_FAILED);
+ fail_unless(errno == ENOMEM);
+
+ fprintf(stdout, " passed\n");
+}
+
int main(int argc, char **argv)
{
char tempname[] = "/tmp/.cmmapXXXXXX";
@@ -476,6 +495,7 @@ int main(int argc, char **argv)
check_file_fixed_mmaps();
check_file_fixed_eof_mmaps();
check_file_unfixed_eof_mmaps();
+ check_invalid_mmaps();
/* Fails at the moment. */
/* check_aligned_anonymous_fixed_mmaps_collide_with_host(); */
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 3/3] linux-user: ppc64: don't use volatile register during safe_syscall
2018-07-31 8:42 [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches Laurent Vivier
2018-07-31 8:42 ` [Qemu-devel] [PULL 1/3] linux-user/mmap.c: handle invalid len maps correctly Laurent Vivier
2018-07-31 8:42 ` [Qemu-devel] [PULL 2/3] tests: add check_invalid_maps to test-mmap Laurent Vivier
@ 2018-07-31 8:42 ` Laurent Vivier
2018-07-31 12:24 ` [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches no-reply
2018-07-31 14:01 ` Peter Maydell
4 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-07-31 8:42 UTC (permalink / raw)
To: qemu-devel
Cc: Riku Voipio, Alex Bennée, Laurent Vivier, Shivaprasad G Bhat
From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
r11 is a volatile register on PPC as per calling conventions.
The safe_syscall code uses it to check if the signal_pending
is set during the safe_syscall. When a syscall is interrupted
on return from signal handling, the r11 might be corrupted
before we retry the syscall leading to a crash. The registers
r0-r13 are not to be used here as they have
volatile/designated/reserved usages.
Change the code to use r14 which is non-volatile.
Use SP+16 which is a slot for LR, for save/restore of previous value
of r14. SP+16 can be used, as LR is preserved across the syscall.
Steps to reproduce:
On PPC host, issue `qemu-x86_64 /usr/bin/cc -E -`
Attempt Ctrl-C, the issue is reproduced.
Reference:
https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG
https://openpowerfoundation.org/wp-content/uploads/2016/03/ABI64BitOpenPOWERv1.1_16July2015_pub4.pdf
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Tested-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <153301568965.30312.10498134581068746871.stgit@dhcp-9-109-246-16>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/host/ppc64/safe-syscall.inc.S | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/linux-user/host/ppc64/safe-syscall.inc.S b/linux-user/host/ppc64/safe-syscall.inc.S
index d30050a67c..8ed73a5b86 100644
--- a/linux-user/host/ppc64/safe-syscall.inc.S
+++ b/linux-user/host/ppc64/safe-syscall.inc.S
@@ -49,7 +49,9 @@ safe_syscall_base:
* and returns the result in r3
* Shuffle everything around appropriately.
*/
- mr 11, 3 /* signal_pending */
+ std 14, 16(1) /* Preserve r14 in SP+16 */
+ .cfi_offset 14, 16
+ mr 14, 3 /* signal_pending */
mr 0, 4 /* syscall number */
mr 3, 5 /* syscall arguments */
mr 4, 6
@@ -67,12 +69,13 @@ safe_syscall_base:
*/
safe_syscall_start:
/* if signal_pending is non-zero, don't do the call */
- lwz 12, 0(11)
+ lwz 12, 0(14)
cmpwi 0, 12, 0
bne- 0f
sc
safe_syscall_end:
/* code path when we did execute the syscall */
+ ld 14, 16(1) /* restore r14 to its original value */
bnslr+
/* syscall failed; return negative errno */
@@ -81,6 +84,7 @@ safe_syscall_end:
/* code path when we didn't execute the syscall */
0: addi 3, 0, -TARGET_ERESTARTSYS
+ ld 14, 16(1) /* restore r14 to its orginal value */
blr
.cfi_endproc
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
2018-07-31 8:42 [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches Laurent Vivier
` (2 preceding siblings ...)
2018-07-31 8:42 ` [Qemu-devel] [PULL 3/3] linux-user: ppc64: don't use volatile register during safe_syscall Laurent Vivier
@ 2018-07-31 12:24 ` no-reply
2018-07-31 12:40 ` Laurent Vivier
2018-07-31 14:01 ` Peter Maydell
4 siblings, 1 reply; 9+ messages in thread
From: no-reply @ 2018-07-31 12:24 UTC (permalink / raw)
To: laurent; +Cc: famz, qemu-devel, riku.voipio, alex.bennee
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180731084203.29959-1-laurent@vivier.eu
Subject: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
806398c875 linux-user: ppc64: don't use volatile register during safe_syscall
ba78346662 tests: add check_invalid_maps to test-mmap
80fc1be868 linux-user/mmap.c: handle invalid len maps correctly
=== OUTPUT BEGIN ===
Checking PATCH 1/3: linux-user/mmap.c: handle invalid len maps correctly...
Checking PATCH 2/3: tests: add check_invalid_maps to test-mmap...
ERROR: code indent should never use tabs
#62: FILE: tests/tcg/multiarch/test-mmap.c:498:
+^Icheck_invalid_mmaps();$
total: 1 errors, 0 warnings, 40 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/3: linux-user: ppc64: don't use volatile register during safe_syscall...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
2018-07-31 12:24 ` [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches no-reply
@ 2018-07-31 12:40 ` Laurent Vivier
2018-07-31 12:44 ` Laurent Vivier
2018-07-31 13:27 ` Alex Bennée
0 siblings, 2 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-07-31 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, riku.voipio, alex.bennee
Le 31/07/2018 à 14:24, no-reply@patchew.org a écrit :
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20180731084203.29959-1-laurent@vivier.eu
> Subject: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
>
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
>
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
>
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> failed=1
> echo
> fi
> n=$((n+1))
> done
>
> exit $failed
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> Switched to a new branch 'test'
> 806398c875 linux-user: ppc64: don't use volatile register during safe_syscall
> ba78346662 tests: add check_invalid_maps to test-mmap
> 80fc1be868 linux-user/mmap.c: handle invalid len maps correctly
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/3: linux-user/mmap.c: handle invalid len maps correctly...
> Checking PATCH 2/3: tests: add check_invalid_maps to test-mmap...
> ERROR: code indent should never use tabs
> #62: FILE: tests/tcg/multiarch/test-mmap.c:498:
> +^Icheck_invalid_mmaps();$
>
> total: 1 errors, 0 warnings, 40 lines checked
I'm going to resend a pull request without the tab.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
2018-07-31 12:40 ` Laurent Vivier
@ 2018-07-31 12:44 ` Laurent Vivier
2018-07-31 13:27 ` Alex Bennée
1 sibling, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-07-31 12:44 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, riku.voipio, alex.bennee
Le 31/07/2018 à 14:40, Laurent Vivier a écrit :
> Le 31/07/2018 à 14:24, no-reply@patchew.org a écrit :
>> Hi,
>>
>> This series seems to have some coding style problems. See output below for
>> more information:
>>
>> Type: series
>> Message-id: 20180731084203.29959-1-laurent@vivier.eu
>> Subject: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>>
>> BASE=base
>> n=1
>> total=$(git log --oneline $BASE.. | wc -l)
>> failed=0
>>
>> git config --local diff.renamelimit 0
>> git config --local diff.renames True
>> git config --local diff.algorithm histogram
>>
>> commits="$(git log --format=%H --reverse $BASE..)"
>> for c in $commits; do
>> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
>> failed=1
>> echo
>> fi
>> n=$((n+1))
>> done
>>
>> exit $failed
>> === TEST SCRIPT END ===
>>
>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>> Switched to a new branch 'test'
>> 806398c875 linux-user: ppc64: don't use volatile register during safe_syscall
>> ba78346662 tests: add check_invalid_maps to test-mmap
>> 80fc1be868 linux-user/mmap.c: handle invalid len maps correctly
>>
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/3: linux-user/mmap.c: handle invalid len maps correctly...
>> Checking PATCH 2/3: tests: add check_invalid_maps to test-mmap...
>> ERROR: code indent should never use tabs
>> #62: FILE: tests/tcg/multiarch/test-mmap.c:498:
>> +^Icheck_invalid_mmaps();$
>>
>> total: 1 errors, 0 warnings, 40 lines checked
>
> I'm going to resend a pull request without the tab.
In fact, no, the whole file uses tabulation. I will not change that.
Peter, could you take the series as-is?
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
2018-07-31 12:40 ` Laurent Vivier
2018-07-31 12:44 ` Laurent Vivier
@ 2018-07-31 13:27 ` Alex Bennée
1 sibling, 0 replies; 9+ messages in thread
From: Alex Bennée @ 2018-07-31 13:27 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, famz, riku.voipio
Laurent Vivier <laurent@vivier.eu> writes:
> Le 31/07/2018 à 14:24, no-reply@patchew.org a écrit:
>> Hi,
>>
>> This series seems to have some coding style problems. See output below for
>> more information:
>>
>> Type: series
>> Message-id: 20180731084203.29959-1-laurent@vivier.eu
>> Subject: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>>
>> BASE=base
>> n=1
>> total=$(git log --oneline $BASE.. | wc -l)
>> failed=0
>>
>> git config --local diff.renamelimit 0
>> git config --local diff.renames True
>> git config --local diff.algorithm histogram
>>
>> commits="$(git log --format=%H --reverse $BASE..)"
>> for c in $commits; do
>> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
>> failed=1
>> echo
>> fi
>> n=$((n+1))
>> done
>>
>> exit $failed
>> === TEST SCRIPT END ===
>>
>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>> Switched to a new branch 'test'
>> 806398c875 linux-user: ppc64: don't use volatile register during safe_syscall
>> ba78346662 tests: add check_invalid_maps to test-mmap
>> 80fc1be868 linux-user/mmap.c: handle invalid len maps correctly
>>
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/3: linux-user/mmap.c: handle invalid len maps correctly...
>> Checking PATCH 2/3: tests: add check_invalid_maps to test-mmap...
>> ERROR: code indent should never use tabs
>> #62: FILE: tests/tcg/multiarch/test-mmap.c:498:
>> +^Icheck_invalid_mmaps();$
Sorry I should of flagged this in the commit message. I left the touched
bits as is and used spaces for new functions.
>>
>> total: 1 errors, 0 warnings, 40 lines checked
>
> I'm going to resend a pull request without the tab.
>
> Thanks,
> Laurent
--
Alex Bennée
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
2018-07-31 8:42 [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches Laurent Vivier
` (3 preceding siblings ...)
2018-07-31 12:24 ` [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches no-reply
@ 2018-07-31 14:01 ` Peter Maydell
4 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2018-07-31 14:01 UTC (permalink / raw)
To: Laurent Vivier; +Cc: QEMU Developers, Riku Voipio, Alex Bennée
On 31 July 2018 at 09:42, Laurent Vivier <laurent@vivier.eu> wrote:
> The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:
>
> Merge remote-tracking branch 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 09:55:47 +0100)
>
> are available in the Git repository at:
>
> git://github.com/vivier/qemu.git tags/linux-user-for-3.0-pull-request
>
> for you to fetch changes up to 5d9f3ea0817215ad4baac5aa30414e9ebbaaf0d6:
>
> linux-user: ppc64: don't use volatile register during safe_syscall (2018-07-31 09:57:43 +0200)
>
> ----------------------------------------------------------------
> Fix safe_syscall() on ppc64 host
> Fix mmap() 0 length error case
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread