From: Prasad J Pandit <pjp@fedoraproject.org>

Limit the number of arguments passed to execve(2) call from
a user program, as large number of them could lead to a bad
guest address error.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 linux-user/syscall.c | 6 ++++++
 1 file changed, 6 insertions(+)

Update per: use gemu_log() to report error
  -> https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00750.html

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9be8e95..86a4a9c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7766,6 +7766,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
     case TARGET_NR_execve:
         {
+#define ARG_MAX 65535
             char **argp, **envp;
             int argc, envc;
             abi_ulong gp;
@@ -7794,6 +7795,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 envc++;
             }
 
+            if (argc > ARG_MAX || envc > ARG_MAX) {
+                gemu_log("argc(%d), envc(%d) exceed %d\n", argc, envc, ARG_MAX);
+                ret = -TARGET_E2BIG;
+                break;
+            }
             argp = alloca((argc + 1) * sizeof(void *));
             envp = alloca((envc + 1) * sizeof(void *));
 
-- 
2.9.3

On 6 March 2017 at 07:17, P J P <ppandit@redhat.com> wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> Limit the number of arguments passed to execve(2) call from
> a user program, as large number of them could lead to a bad
> guest address error.
>
> Reported-by: Jann Horn <jannh@google.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  linux-user/syscall.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> Update per: use gemu_log() to report error
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00750.html
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9be8e95..86a4a9c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7766,6 +7766,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #endif
>      case TARGET_NR_execve:
>          {
> +#define ARG_MAX 65535
>              char **argp, **envp;
>              int argc, envc;
>              abi_ulong gp;
> @@ -7794,6 +7795,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  envc++;
>              }
>
> +            if (argc > ARG_MAX || envc > ARG_MAX) {
> +                gemu_log("argc(%d), envc(%d) exceed %d\n", argc, envc, ARG_MAX);
> +                ret = -TARGET_E2BIG;
> +                break;
> +            }
>              argp = alloca((argc + 1) * sizeof(void *));
>              envp = alloca((envc + 1) * sizeof(void *));


We need to fix this by not using alloca(), not by imposing
an arbitrary limit that's still rather over-large for an
alloca allocation, as Eric suggested.

thanks
-- PMM

On 03/06/2017 09:42 AM, Peter Maydell wrote:
> On 6 March 2017 at 07:17, P J P <ppandit@redhat.com> wrote:
>> From: Prasad J Pandit <pjp@fedoraproject.org>
>>
>> Limit the number of arguments passed to execve(2) call from
>> a user program, as large number of them could lead to a bad
>> guest address error.
>>
>> Reported-by: Jann Horn <jannh@google.com>
>> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
>> ---
>>  linux-user/syscall.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>

>>          {
>> +#define ARG_MAX 65535
>>              char **argp, **envp;
>>              int argc, envc;
>>              abi_ulong gp;
>> @@ -7794,6 +7795,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>                  envc++;
>>              }
>>
>> +            if (argc > ARG_MAX || envc > ARG_MAX) {
>> +                gemu_log("argc(%d), envc(%d) exceed %d\n", argc, envc, ARG_MAX);
>> +                ret = -TARGET_E2BIG;
>> +                break;
>> +            }
>>              argp = alloca((argc + 1) * sizeof(void *));
>>              envp = alloca((envc + 1) * sizeof(void *));
> 
> 
> We need to fix this by not using alloca(), not by imposing
> an arbitrary limit that's still rather over-large for an
> alloca allocation, as Eric suggested.

And patch 2/2 does that. Does that patch in isolation fix the problem?
In which case, we don't want this patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org