| Message ID | 20170306071721.26708-2-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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 *));