diff mbox series

[v2,for,3.0,1/2] linux-user/mmap.c: handle invalid len maps correctly

Message ID 20180730134321.19898-2-alex.bennee@linaro.org
State New
Headers show
Series fix for bug #1783362 | expand

Commit Message

Alex Bennée July 30, 2018, 1:43 p.m. UTC 
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(-)

-- 
2.17.1

Comments

Laurent Vivier July 30, 2018, 2:12 p.m. UTC | #1 
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>
Alex Bennée July 30, 2018, 2:21 p.m. UTC | #2 
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
Laurent Vivier July 30, 2018, 2:30 p.m. UTC | #3 
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
Richard Henderson July 30, 2018, 9:42 p.m. UTC | #4 
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~
diff mbox series

Patch

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;