[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