On Thu, Sep 03, 2015 at 04:33:42PM -0700, Andy Zhou wrote: > Allow daemon running as root to accept --user option, that accepts > "user:group" string as input. Performs sanity check on the input, > and store the converted uid and gid. > > daemon_become_new_user() needs to be called to make the actual > switch. > > Signed-off-by: Andy Zhou <az...@nicira.com>
Thanks for implementing this. There should be a new-line after void here: > +void daemon_set_new_user(const char *user_spec) > +{ I think the ability to use setuid() and friends only depends on having uid 0, not gid 0: > + uid = getuid(); > + gid = getgid(); > + > + if (gid || uid) { > + VLOG_FATAL("%s: only root can use --user option", pidfile); > + } Here in daemon_set_new_user() I would use xmemdup0() instead: > + if (len) { > + user = xzalloc(len + 1); > + strncpy(user, user_spec, len); and this should be xstrdup(): > + user = strdup(pwd.pw_name); > + } I think that the parsing in daemon_set_new_user() assumes that white space, if present, will precede ':'. If not, then 'len' will end up negative, which looks bad to me. I think I'd just not bother worrying about white space in the parameter. I am not sure that it is valuable to check that the user belongs to the specified group. I don't think that other software tends to perform such a check; I don't see one in Apache, for example. I might have other comments when I look at the final patch. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev