| Message ID | 1447101793-13804-1-git-send-email-azhou@nicira.com |
|---|---|
| State | Rejected |
| Headers | show |
On 9 November 2015 at 12:43, Andy Zhou <azhou@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@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@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
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"); + } } 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; }
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@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(-)