[ovs-dev] [additional --user changes v2 1/3] lib: simplify daemon_become_new_user__()

Ansis Atteka ansisatteka at gmail.com
Tue Nov 10 00:00:30 UTC 2015


On 9 November 2015 at 12:43, Andy Zhou <azhou at nicira.com> wrote:

> A global variable 'switch_user' was used to make sure
> we switch process's current user only once. This logic is now
> simplified by testing for uid directly; if switch process has
> taken place, the current uid will be not be zero.
>
> Signed-off-by: Andy Zhou <azhou at nicira.com>
>
> ---
> v1->v2:  add a log in case --user is specified but not switched.
> ---
>  lib/daemon-unix.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index 868e2c9..645a3ac 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -83,7 +83,6 @@ static bool monitor;
>
>  /* --user: Only root can use this option. Switch to new uid:gid after
>   * initially running as root.  */
> -static bool switch_user = false;
>  static bool non_root_user = false;
>  static uid_t uid;
>  static gid_t gid;
> @@ -440,14 +439,14 @@ daemonize_start(bool access_datapath)
>      assert_single_threaded();
>      daemonize_fd = -1;
>
> -    if (switch_user) {
> +    if (non_root_user) {
>          daemon_become_new_user__(access_datapath);
> -        switch_user = false;
> -    }
>
> -    /* If --user is specified, make sure user switch has completed by
> now.  */
> -    if (non_root_user) {
> -        ovs_assert(geteuid() && getuid());
> +        /* If --user is specified, make sure user switch has completed
> +         * by now.  */
> +        if (geteuid() && getuid()) {
> +            ovs_fatal(0, "--user is specifed, but not switched \n");
>
I am still not 100% convinced that the best behavior here is to fail the
program. I would think that the best behavior is to do nothing if
ovs-vswitchd was started as root and received "--user root:root" argument.

Can you catch such errors where user switch failed from
daemon_become_new_user__() instead?

+        }
>      }
>
>      if (detach) {
> @@ -853,6 +852,12 @@ daemon_become_new_user_linux(bool access_datapath
> OVS_UNUSED)
>  static void
>  daemon_become_new_user__(bool access_datapath)
>  {
> +    /* Make sure we exectue this function at most once as a root
> +     * process. */
> +    if (getuid() || geteuid()) {
> +        return;
> +    }
> +
>      if (LINUX) {
>          if (LIBCAPNG) {
>              daemon_become_new_user_linux(access_datapath);
> @@ -873,12 +878,8 @@ void
>  daemon_become_new_user(bool access_datapath)
>  {
>      assert_single_threaded();
> -    if (switch_user) {
> +    if (non_root_user) {
>          daemon_become_new_user__(access_datapath);
> -
> -        /* Make sure daemonize_start() will not switch
> -         * user again. */
> -        switch_user = false;
>      }
>  }
>
> @@ -1041,5 +1042,5 @@ daemon_set_new_user(const char *user_spec)
>          }
>      }
>
> -    switch_user = non_root_user = true;
> +    non_root_user = true;
>  }
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list