Re: [Qemu-devel] [PATCH] virtfs-proxy-helper: check return code of setfsgid/setfsuid

[Stefan Weil <sw@weilnetz.de>]

Am 10.10.2012 18:36, schrieb Paolo Bonzini:
> Il 10/10/2012 18:23, Stefan Weil ha scritto:
>> < 0 would be wrong because it looks like both functions never
>> return negative values.
>> I just wrote a small test program (see
>> below) and called it with different uids with and without root
>> rights. This pattern should be fine:
>>
>> new_uid = setfsuid(uid);
>> if (new_uid != 0 && new_uid != uid) {
>>    return -1;
>> }
> I didn't really care about this case.  I assumed that the authors knew
> what they were doing...
>
> What I cared about is: "When glibc determines that the argument is not a
>   valid  group  ID,  it will  return  -1  and set errno to EINVAL without
> attempting the system call".

I was not able to get -1 with my test program: any value which I tried
seemed to work when the program was called with sudo.

>
> I think this would also work:
>
>     if (setfsuid(uid) < 0 || setfsuid(uid) != uid) {
>         return -1;
>     }
>
> but it seems wasteful to do four syscalls instead of two.
>
> Paolo

I added a local variable in my example to avoid those extra
syscalls.

Your last patch v2 does not handle missing rights (no root)
because in that case the functions don't return a value < 0
but fail nevertheless.Calling a program which requires
root privileges from a normal user account is usually a
very common error. I don't know the use cases for virtfs -
maybe that's no problem here.

The functions have an additional problem: they don't set
errno (see manpages). I tested this, and here the manpages
are correct. The code in virtfs-proxy-helper expects that
errno was set, so the patch must set errno = EPERM or
something like that.

Stefan