Stefan Weil <s...@weilnetz.de> writes:
We need to change fsuid and fsgid in 9p server side when 9p client wants
to create a file with given uid and gid.
In my case setfsuid and setfsgid never return -1 even if a normal user tries to
change fsuid.
I am running F17 and glibc is 2.15-56.fc17
IMHO setfsuid/setfsgid need to return -1 & set errno to EPERM when calling user
lacks CAP_SETUID capability. I can work on the kernel side to return
proper error values.
Also as per the man page:
When glibc determines that the argument is not a valid user ID,
it will return -1 and set errno to EINVAL
without attempting the system call.
If it mean a nonexistent id by 'not a valid user ID' it may be a
problem in virtfs case. Client sends the user id details which may be
a nonexistent user id in host system. Ideally setfsuid setfsgid sets
whatever uid/gid passed if the caller is root as of now.
> Am 10.10.2012 18:54, schrieb Stefan Weil:
>> 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
>
> Maybe the author of those code can tell us more on the
> use cases and which errors must be handled.
>
> Is it necessary to use those functions at all (they are very
> Linux specific), or can they be replaced by seteuid, setegid?
>
> Regards
>
> Stefan W.
