Christian Ehrhardt <christian.ehrha...@canonical.com> writes:
> 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>
>> ---
Cool! From my recent series (in the ovs_strtousr function):
+ if (e != 0 || !res) {
+ if (!res) e = ENOENT;
+ VLOG_ERR("Failed to retrieve user pwentry (%s), aborting.",
+ ovs_strerror(e));
+ goto release;
+ }
I think I did testing with the ovs_strtousr function, and figured I
missed the check when moving it over.
>> 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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
