|
From: | Stefan Weil |
Subject: | Re: [Qemu-devel] [PATCH] virtfs-proxy-helper: check return code of setfsgid/setfsuid |
Date: | Wed, 10 Oct 2012 18:54:27 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120912 Thunderbird/15.0.1 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |