Aarons last patch reminded me that we didn't went on on the fixed patch for
this code path that Aaron just modified.
So giving this a bump to show up again.
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
On Mon, Apr 25, 2016 at 2:12 AM, Christian Ehrhardt <
christian.ehrha...@canonical.com> wrote:
> From the manpages of getgrnam_r (getpwnam_r is similar):
> "If no matching group record was found, these functions return 0 and
> store NULL in *result."
>
> The code checked only against errors, but non existing users didn't set
> e != 0 therefore the code could try to set arbitrary uid/gid values.
>
> *Update in v2*
> fix wrong pointer usage of *res and running full set of unit tests to be
> sure.
>
> Fixes: e91b927d lib/daemon: support --user option for all OVS daemon
>
> Signed-off-by: Christian Ehrhardt <christian.ehrha...@canonical.com>
> ---
> lib/daemon-unix.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index 182f76b..28f76da 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -972,6 +972,9 @@ daemon_set_new_user(const char *user_spec)
> VLOG_FATAL("%s: Failed to retrive user %s's uid (%s),
> aborting.",
> pidfile, user, ovs_strerror(e));
> }
> + if (res == NULL) {
> + VLOG_FATAL("%s: user %s not found, aborting.", pidfile, user);
> + }
> } else {
> /* User name is not specified, use current user. */
> while ((e = getpwuid_r(uid, &pwd, buf, bufsize, &res)) == ERANGE)
> {
> @@ -1012,6 +1015,10 @@ daemon_set_new_user(const char *user_spec)
> "(%s), aborting.", pidfile, grpstr,
> ovs_strerror(e));
> }
> + if (res == NULL) {
> + VLOG_FATAL("%s: group %s not found, aborting.", pidfile,
> + grpstr);
> + }
>
> if (gid != grp.gr_gid) {
> char **mem;
> --
> 2.7.4
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
