On Wed, Nov 11, 2015 at 2:48 PM, Ansis Atteka <[email protected]> wrote: > > > On 11 November 2015 at 14:13, Andy Zhou <[email protected]> wrote: >> >> vlog log file can be created when parsing --log-file option, before >> switching user, in case the --user option is also specified. While this >> does not directly cause errors for the running daemons, it can >> leave the log files on the disk as created under the "root" user. >> This patch fix the log file ownership to the user specified with --user. >> >> Signed-off-by: Andy Zhou <[email protected]> >> >> --- >> v1->v2: Add a comment on vlog_change_owner return code. >> v2->v3: no change >> v3->v4: Reword the commit message. change vlog_change_owner() to void. >> --- >> include/openvswitch/vlog.h | 1 + >> lib/daemon-unix.c | 6 +++++- >> lib/vlog.c | 22 +++++++++++++++++++++- >> 3 files changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h >> index f6bb3ab..bc16590 100644 >> --- a/include/openvswitch/vlog.h >> +++ b/include/openvswitch/vlog.h >> @@ -143,6 +143,7 @@ void vlog_set_verbosity(const char *arg); >> void vlog_set_pattern(enum vlog_destination, const char *pattern); >> int vlog_set_log_file(const char *file_name); >> int vlog_reopen_log_file(void); >> +void vlog_change_owner(uid_t, gid_t); >> >> /* Configure method how vlog should send messages to syslog server. */ >> void vlog_set_syslog_method(const char *method); >> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c >> index 0125745..e69517a 100644 >> --- a/lib/daemon-unix.c >> +++ b/lib/daemon-unix.c >> @@ -739,7 +739,7 @@ daemon_switch_group(gid_t real, gid_t effective, >> { >> if ((setresgid(real, effective, saved) == -1) || >> !gid_verify(real, effective, saved)) { >> - VLOG_FATAL("%s: fail to switch group to gid as %d, aborting", >> + VLOG_FATAL("%s: failed to switch group to gid as %d, aborting", >> pidfile, gid); >> } >> } >> @@ -847,6 +847,10 @@ daemon_become_new_user_linux(bool access_datapath >> OVS_UNUSED) >> static void >> daemon_become_new_user__(bool access_datapath) >> { >> + /* If vlog file has been created, change its owner to the non-root >> user >> + * as specifed by the --user option. */ >> + vlog_change_owner(uid, gid); >> + >> if (LINUX) { >> if (LIBCAPNG) { >> daemon_become_new_user_linux(access_datapath); >> diff --git a/lib/vlog.c b/lib/vlog.c >> index da31e6f..ade0a45 100644 >> --- a/lib/vlog.c >> +++ b/lib/vlog.c >> @@ -105,7 +105,7 @@ DEFINE_STATIC_PER_THREAD_DATA(unsigned int, msg_num, >> 0); >> * All of the following is protected by 'log_file_mutex', which nests >> inside >> * pattern_rwlock. */ >> static struct ovs_mutex log_file_mutex = OVS_MUTEX_INITIALIZER; >> -static char *log_file_name OVS_GUARDED_BY(log_file_mutex); >> +static char *log_file_name = NULL OVS_GUARDED_BY(log_file_mutex); >> static int log_fd OVS_GUARDED_BY(log_file_mutex) = -1; >> static struct async_append *log_writer OVS_GUARDED_BY(log_file_mutex); >> static bool log_async OVS_GUARDED_BY(log_file_mutex); >> @@ -430,6 +430,26 @@ vlog_reopen_log_file(void) >> } >> } >> >> +/* In case a log file exists, change its owner to new 'user' and 'group'. >> + * >> + * This is useful for handling cases where the --log-file option is >> + * specified ahead of the --user option. */ >> +void >> +vlog_change_owner(uid_t user, gid_t group) >> +{ >> + int error = 0; >> + >> + if (log_file_name) { >> + ovs_mutex_lock(&log_file_mutex); >> + error = chown(log_file_name, user, group); >> + ovs_mutex_unlock(&log_file_mutex); >> + } >> + >> + if (error) { >> + VLOG_FATAL("Failed to change log file ownership."); > > I would print errno value here and the file name you are actually trying to > change the ownership for. It would simply provide a hint to the users on > what was actually wrong, if it failed. > > VLOG_FATAL("Failed to change %s ownership: %s", log_file_name, > ovs_strerror(errno)); > > And early return from function if log_file_name is NULL to make code look > better. >
Pushed to master with both changes as suggested. >> + } >> +} >> + > > Otherwise, Acked-by: Ansis Atteka <[email protected]> > > Thanks for working on this, Andy. Thanks for your review. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
