[pulseaudio-discuss] [PATCH] daemon: Don't rely on prctl(PR_SET_KEEPCAPS, 0) for dropping caps.

[Tanu Kaskinen]

Does someone want to review this?


On Thu, 2012-04-05 at 15:37 +0300, Tanu Kaskinen wrote:
> Capability dropping when changing the user in the system
> mode was previously implemented by calling
> prctl(PR_SET_KEEPCAPS, 0), but that doesn't necessarily
> work. It's possible that the KEEPCAPS flag is locked to 1,
> in which case the prctl() call fails with EPERM (this
> happens at least on Harmattan). This patch implements
> explicit capability dropping after changing the user.
> ---
>  src/daemon/caps.c |   27 +++++++++++++--------------
>  src/daemon/caps.h |    2 ++
>  src/daemon/main.c |    4 +++-
>  3 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/src/daemon/caps.c b/src/daemon/caps.c
> index 3759388..36b76a9 100644
> --- a/src/daemon/caps.c
> +++ b/src/daemon/caps.c
> @@ -36,10 +36,6 @@
>  #include <sys/capability.h>
>  #endif
>  
> -#ifdef HAVE_SYS_PRCTL_H
> -#include <sys/prctl.h>
> -#endif
> -
>  #include "caps.h"
>  
>  /* Glibc <= 2.2 has broken unistd.h */
> @@ -78,17 +74,20 @@ void pa_drop_root(void) {
>      pa_assert_se(getegid() == gid);
>  #endif
>  
> -#ifdef HAVE_SYS_PRCTL_H
> -    pa_assert_se(prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) == 0);
> -#endif
> +    if (uid != 0)
> +        pa_drop_caps();
> +}
>  
> +void pa_drop_caps(void) {
>  #ifdef HAVE_SYS_CAPABILITY_H
> -    if (uid != 0) {
> -        cap_t caps;
> -        pa_assert_se(caps = cap_init());
> -        pa_assert_se(cap_clear(caps) == 0);
> -        pa_assert_se(cap_set_proc(caps) == 0);
> -        pa_assert_se(cap_free(caps) == 0);
> -    }
> +    cap_t caps;
> +    pa_assert_se(caps = cap_init());
> +    pa_assert_se(cap_clear(caps) == 0);
> +    pa_assert_se(cap_set_proc(caps) == 0);
> +    pa_assert_se(cap_free(caps) == 0);
> +#else
> +    pa_log_warn("Normally all extra capabilities would be dropped now, but "
> +                "that's impossible because this Pulseaudio was built without "
> +                "libcap support.");
>  #endif
>  }
> diff --git a/src/daemon/caps.h b/src/daemon/caps.h
> index 5d0ee62..e9cd7cb 100644
> --- a/src/daemon/caps.h
> +++ b/src/daemon/caps.h
> @@ -26,4 +26,6 @@
>  
>  void pa_drop_root(void);
>  
> +void pa_drop_caps(void);
> +
>  #endif
> diff --git a/src/daemon/main.c b/src/daemon/main.c
> index da1e0cf..0ba51cd 100644
> --- a/src/daemon/main.c
> +++ b/src/daemon/main.c
> @@ -251,6 +251,8 @@ static int change_user(void) {
>          return -1;
>      }
>  
> +    pa_drop_caps();
> +
>      pa_set_env("USER", PA_SYSTEM_USER);
>      pa_set_env("USERNAME", PA_SYSTEM_USER);
>      pa_set_env("LOGNAME", PA_SYSTEM_USER);
> @@ -266,7 +268,7 @@ static int change_user(void) {
>      if (!getenv("PULSE_STATE_PATH"))
>          pa_set_env("PULSE_STATE_PATH", PA_SYSTEM_STATE_PATH);
>  
> -    pa_log_info(_("Successfully dropped root privileges."));
> +    pa_log_info(_("Successfully changed user to \"" PA_SYSTEM_USER "\"."));
>  
>      return 0;
>  }

-- 
Tanu