* [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362
@ 2018-07-30 13:43 Alex Bennée
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly Alex Bennée
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Alex Bennée @ 2018-07-30 13:43 UTC (permalink / raw)
To: qemu-devel; +Cc: 1783362, Alex Bennée
Hi,
Updated to cover the overflow case properly as well.
Alex Bennée (2):
linux-user/mmap.c: handle invalid len maps correctly
tests: add check_invalid_maps to test-mmap
linux-user/mmap.c | 15 ++++++++++++---
tests/tcg/multiarch/test-mmap.c | 22 +++++++++++++++++++++-
2 files changed, 33 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly
2018-07-30 13:43 [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 Alex Bennée
@ 2018-07-30 13:43 ` Alex Bennée
2018-07-30 14:12 ` Laurent Vivier
2018-07-30 21:42 ` Richard Henderson
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 2/2] tests: add check_invalid_maps to test-mmap Alex Bennée
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Alex Bennée @ 2018-07-30 13:43 UTC (permalink / raw)
To: qemu-devel; +Cc: 1783362, Alex Bennée, Riku Voipio, Laurent Vivier
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>
---
v2
- add comment on overflow
---
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] [PATCH v2 for 3.0 2/2] tests: add check_invalid_maps to test-mmap
2018-07-30 13:43 [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 Alex Bennée
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly Alex Bennée
@ 2018-07-30 13:43 ` Alex Bennée
2018-07-30 15:24 ` [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 no-reply
2018-07-31 4:08 ` no-reply
3 siblings, 0 replies; 9+ messages in thread
From: Alex Bennée @ 2018-07-30 13:43 UTC (permalink / raw)
To: qemu-devel; +Cc: 1783362, Alex Bennée
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>
---
v2
- add test for overflow
---
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
* Re: [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly Alex Bennée
@ 2018-07-30 14:12 ` Laurent Vivier
2018-07-30 14:21 ` Alex Bennée
2018-07-30 21:42 ` Richard Henderson
1 sibling, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2018-07-30 14:12 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, 1783362
Le 30/07/2018 à 15:43, Alex Bennée a écrit :
> 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>
>
> ---
> v2
> - add comment on overflow
> ---
> 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;
>
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly
2018-07-30 14:12 ` Laurent Vivier
@ 2018-07-30 14:21 ` Alex Bennée
2018-07-30 14:30 ` Laurent Vivier
0 siblings, 1 reply; 9+ messages in thread
From: Alex Bennée @ 2018-07-30 14:21 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, Riku Voipio, 1783362
Laurent Vivier <laurent@vivier.eu> writes:
> Le 30/07/2018 à 15:43, Alex Bennée a écrit:
>> 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>
>>
>> ---
>> v2
>> - add comment on overflow
>> ---
>> 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;
>>
>>
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Are you going to take this via your queue or do you want me to re-post
with the r-b?
--
Alex Bennée
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly
2018-07-30 14:21 ` Alex Bennée
@ 2018-07-30 14:30 ` Laurent Vivier
0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-07-30 14:30 UTC (permalink / raw)
To: Alex Bennée; +Cc: Riku Voipio, 1783362, qemu-devel
Le 30/07/2018 à 16:21, Alex Bennée a écrit :
>
> Laurent Vivier <laurent@vivier.eu> writes:
>
>> Le 30/07/2018 à 15:43, Alex Bennée a écrit:
>>> 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>
>>>
>>> ---
>>> v2
>>> - add comment on overflow
>>> ---
>>> 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;
>>>
>>>
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>
> Are you going to take this via your queue or do you want me to re-post
> with the r-b?
I can take this via my queue.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362
2018-07-30 13:43 [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 Alex Bennée
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly Alex Bennée
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 2/2] tests: add check_invalid_maps to test-mmap Alex Bennée
@ 2018-07-30 15:24 ` no-reply
2018-07-31 4:08 ` no-reply
3 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2018-07-30 15:24 UTC (permalink / raw)
To: alex.bennee; +Cc: famz, qemu-devel, 1783362
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180730134321.19898-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362
=== 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'
4615f64664 tests: add check_invalid_maps to test-mmap
485d98a9f6 linux-user/mmap.c: handle invalid len maps correctly
=== OUTPUT BEGIN ===
Checking PATCH 1/2: linux-user/mmap.c: handle invalid len maps correctly...
Checking PATCH 2/2: tests: add check_invalid_maps to test-mmap...
ERROR: code indent should never use tabs
#61: 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.
=== 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] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly Alex Bennée
2018-07-30 14:12 ` Laurent Vivier
@ 2018-07-30 21:42 ` Richard Henderson
1 sibling, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2018-07-30 21:42 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, 1783362, Laurent Vivier
On 07/30/2018 09:43 AM, Alex Bennée wrote:
> 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>
>
> ---
> v2
> - add comment on overflow
> ---
> linux-user/mmap.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362
2018-07-30 13:43 [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 Alex Bennée
` (2 preceding siblings ...)
2018-07-30 15:24 ` [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 no-reply
@ 2018-07-31 4:08 ` no-reply
3 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2018-07-31 4:08 UTC (permalink / raw)
To: alex.bennee; +Cc: famz, qemu-devel, 1783362
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180730134321.19898-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362
=== 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'
b48e0e0795 tests: add check_invalid_maps to test-mmap
b531edb72a linux-user/mmap.c: handle invalid len maps correctly
=== OUTPUT BEGIN ===
Checking PATCH 1/2: linux-user/mmap.c: handle invalid len maps correctly...
Checking PATCH 2/2: tests: add check_invalid_maps to test-mmap...
ERROR: code indent should never use tabs
#61: 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.
=== 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
end of thread, other threads:[~2018-07-31 19:54 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30 13:43 [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 Alex Bennée
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly Alex Bennée
2018-07-30 14:12 ` Laurent Vivier
2018-07-30 14:21 ` Alex Bennée
2018-07-30 14:30 ` Laurent Vivier
2018-07-30 21:42 ` Richard Henderson
2018-07-30 13:43 ` [Qemu-devel] [PATCH v2 for 3.0 2/2] tests: add check_invalid_maps to test-mmap Alex Bennée
2018-07-30 15:24 ` [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362 no-reply
2018-07-31 4:08 ` no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.