| Message ID | 20230609162915.175810-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] linux-user: Return EINVAL for getgroups() with negative gidsetsize | expand |
09.06.2023 19:29, Peter Maydell wrote: > Coverity doesn't like the way we might end up calling getgroups() > with a NULL grouplist pointer. This is fine for the special case > of gidsetsize == 0, but we will also do it if the guest passes > us a negative gidsetsize. (CID 1512465) > > Explicitly fail the negative gidsetsize with EINVAL, as the kernel > does. This means we definitely only call the libc getgroups() > with valid parameters. It also brings the getgroups() code in > to line with the setgroups() code. > > Possibly Coverity may still complain about getgroups(0, NULL), but > that would be a false positive. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > v2: also change TARGET_NR_getgroups32 code Reviewed-by: Michael Tokarev <mjt@tls.msk.ru> This can be applied to -trivial just fine, I think. There's another change in there already in exactly this area. Thank you Peter! /mjt
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 83685f0aa59..bb29444ca7e 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -11676,7 +11676,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, g_autofree gid_t *grouplist = NULL; int i; - if (gidsetsize > NGROUPS_MAX) { + if (gidsetsize > NGROUPS_MAX || gidsetsize < 0) { return -TARGET_EINVAL; } if (gidsetsize > 0) { @@ -12012,7 +12012,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, g_autofree gid_t *grouplist = NULL; int i; - if (gidsetsize > NGROUPS_MAX) { + if (gidsetsize > NGROUPS_MAX || gidsetsize < 0) { return -TARGET_EINVAL; } if (gidsetsize > 0) {
Coverity doesn't like the way we might end up calling getgroups() with a NULL grouplist pointer. This is fine for the special case of gidsetsize == 0, but we will also do it if the guest passes us a negative gidsetsize. (CID 1512465) Explicitly fail the negative gidsetsize with EINVAL, as the kernel does. This means we definitely only call the libc getgroups() with valid parameters. It also brings the getgroups() code in to line with the setgroups() code. Possibly Coverity may still complain about getgroups(0, NULL), but that would be a false positive. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- v2: also change TARGET_NR_getgroups32 code --- linux-user/syscall.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)