"Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> writes:
> I found this to be confusing. How about avoiding all those pointers, something
> like below ? If you are ok can I add the signed-off-by for this ? I can
> test this and get a pull request out with the build fix.
>
> commit 24cc9f0d07c2a505bfafbdcb72006f2eda1288a4
> Author: Paolo Bonzini <pbon...@redhat.com>
> Date: Thu Oct 11 14:20:23 2012 +0200
>
> virtfs-proxy-helper: use setresuid and setresgid
>
> The setfsuid and setfsgid system calls are obscure and they complicate
> the error checking (that glibc's warn_unused_result "feature" forces
> us to do). Switch to the standard setresuid and setresgid functions.
>
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index f9a8270..49ab0eb 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -272,31 +272,59 @@ static int send_status(int sockfd, struct iovec *iovec,
> int status)
> /*
> * from man 7 capabilities, section
> * Effect of User ID Changes on Capabilities:
> - * 4. If the file system user ID is changed from 0 to nonzero (see
> setfsuid(2))
> - * then the following capabilities are cleared from the effective set:
> - * CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH, CAP_FOWNER, CAP_FSETID,
> - * CAP_LINUX_IMMUTABLE (since Linux 2.2.30), CAP_MAC_OVERRIDE, and
> CAP_MKNOD
> - * (since Linux 2.2.30). If the file system UID is changed from nonzero to 0,
> - * then any of these capabilities that are enabled in the permitted set
> - * are enabled in the effective set.
> + * If the effective user ID is changed from nonzero to 0, then the permitted
> + * set is copied to the effective set. If the effective user ID is changed
> + * from 0 to nonzero, then all capabilities are are cleared from the
> effective
> + * set.
> + *
> + * The setfsuid/setfsgid man pages warn that changing the effective user ID
> may
> + * expose the program to unwanted signals, but this is not true anymore: for
> an
> + * unprivileged (without CAP_KILL) program to send a signal, the real or
> + * effective user ID of the sending process must equal the real or saved user
> + * ID of the target process. Even when dropping privileges, it is enough to
> + * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
> + * be exposed to signals. So just use setresuid/setresgid.
> */
> -static int setfsugid(int uid, int gid)
> +static int setugid(int uid, int gid, int suid, int sgid)
> {
> + int retval;
> +
> /*
> - * We still need DAC_OVERRIDE because we don't change
> + * We still need DAC_OVERRIDE because we don't change
> * supplementary group ids, and hence may be subjected DAC rules
> */
> cap_value_t cap_list[] = {
> CAP_DAC_OVERRIDE,
> };
>
> - setfsgid(gid);
> - setfsuid(uid);
> + if (setresuid(-1, uid, suid) == -1) {
> + retval = -errno;
> + goto err_out;
> + }
> + if (setresgid(-1, gid, sgid) == -1) {
> + retval = -errno;
> + goto err_suid;
> + }
>
After changing the order of setresuid and setresgid this patch works as
expected. Please move setresgid before setresuid.
Tested-by: M. Mohan Kumar <mo...@in.ibm.com>
