[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl: Be more careful with error handling in libxl__dm_runas_helper()
On Wed, 2015-11-25 at 16:56 -0500, Boris Ostrovsky wrote: > getpwnam_r() has fairly complicated return rules. From man pages: > > Â RETURN VALUE > ÂÂÂÂÂÂ... > ÂÂÂÂÂÂOn success, getpwnam_r() and getpwuid_r() return zero, and set > ÂÂÂÂÂÂ*result to pwd.ÂÂIf no matchingÂÂpassword record was found, these > ÂÂÂÂÂÂfunctions return 0 and store NULL in *result. In case of error, > ÂÂÂÂÂÂan error number is returned, and NULL is stored in *result. > Â ERRORS > ÂÂÂÂÂÂ0 or ENOENT or ESRCH or EBADF or EPERM or ... > ÂÂÂÂÂÂÂÂÂÂÂÂThe given name or uid was not found. My reference when reviewing this is the (IMHO more canonical)Âhttp://pubs.o pengroup.org/onlinepubs/9699919799/functions/getpwnam.htmlÂ. I suppose you are looking at the Linux and/or glibc man pages? > While it's not clear what ellipses are meant to be, the way we currently > treat return values from getpwnam_r() is no sufficient. In fact, two of > my systems behave differently when username is not found: one returns > ENOENT and the other returns 0. Which two systems are these? When you say "returns" do you mean "returns 0 and sets errno to XXX" or literally returns ENOENT? > Both set *result to NULL. > > This patch adjusts return value management to be more in line with man > pages. > > While at it, also make sure we don't get stuck on ERANGE. > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> > --- > Âtools/libxl/libxl_dm.c | 23 ++++++++++++++--------- > Â1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index a4934df..bd3daeb 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -726,7 +726,7 @@ static int libxl__dm_runas_helper(libxl__gc *gc, > const char *username) > ÂÂÂÂÂstruct passwd pwd, *user = NULL; > ÂÂÂÂÂchar *buf = NULL; > ÂÂÂÂÂlong buf_size; > -ÂÂÂÂint ret; > +ÂÂÂÂint ret, retry_cnt = 0; > Â > ÂÂÂÂÂbuf_size = sysconf(_SC_GETPW_R_SIZE_MAX); > ÂÂÂÂÂif (buf_size < 0) { > @@ -740,12 +740,17 @@ static int libxl__dm_runas_helper(libxl__gc *gc, > const char *username) > ÂÂÂÂÂÂÂÂÂret = getpwnam_r(username, &pwd, buf, buf_size, &user); > ÂÂÂÂÂÂÂÂÂif (ret == ERANGE) { > ÂÂÂÂÂÂÂÂÂÂÂÂÂbuf_size += 128; > +ÂÂÂÂÂÂÂÂÂÂÂÂif (retry_cnt++ > 10) > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn ERROR_FAIL; > ÂÂÂÂÂÂÂÂÂÂÂÂÂcontinue; > ÂÂÂÂÂÂÂÂÂ} > -ÂÂÂÂÂÂÂÂif (ret != 0) > -ÂÂÂÂÂÂÂÂÂÂÂÂreturn ERROR_FAIL; > -ÂÂÂÂÂÂÂÂif (user != NULL) > -ÂÂÂÂÂÂÂÂÂÂÂÂreturn 1; > +ÂÂÂÂÂÂÂÂif (user == NULL) { > +ÂÂÂÂÂÂÂÂÂÂÂÂif (!ret || (ret == ENOENT) || (ret == ESRCH) || > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ(ret == EBADF) || (ret == EPERM)) > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn ERROR_NOTFOUND; > +ÂÂÂÂÂÂÂÂÂÂÂÂelse > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn ERROR_FAIL; > +ÂÂÂÂÂÂÂÂ} > ÂÂÂÂÂÂÂÂÂreturn 0; > ÂÂÂÂÂ} > Â} > @@ -1261,16 +1266,16 @@ static int > libxl__build_device_model_args_new(libxl__gc *gc, > Â > ÂÂÂÂÂÂÂÂÂuser = GCSPRINTF("%s%d", LIBXL_QEMU_USER_BASE, guest_domid); > ÂÂÂÂÂÂÂÂÂret = libxl__dm_runas_helper(gc, user); > -ÂÂÂÂÂÂÂÂif (ret < 0) > +ÂÂÂÂÂÂÂÂif (ret && (ret != ERROR_NOTFOUND)) > ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn ret; > -ÂÂÂÂÂÂÂÂif (ret > 0) > +ÂÂÂÂÂÂÂÂif (!ret) > ÂÂÂÂÂÂÂÂÂÂÂÂÂgoto end_search; > Â > ÂÂÂÂÂÂÂÂÂuser = LIBXL_QEMU_USER_SHARED; > ÂÂÂÂÂÂÂÂÂret = libxl__dm_runas_helper(gc, user); > -ÂÂÂÂÂÂÂÂif (ret < 0) > +ÂÂÂÂÂÂÂÂif (ret && (ret != ERROR_NOTFOUND)) > ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn ret; > -ÂÂÂÂÂÂÂÂif (ret > 0) { > +ÂÂÂÂÂÂÂÂif (!ret) { > ÂÂÂÂÂÂÂÂÂÂÂÂÂLOG(WARN, "Could not find user %s%d, falling back to %s", > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂLIBXL_QEMU_USER_BASE, guest_domid, > LIBXL_QEMU_USER_SHARED); > ÂÂÂÂÂÂÂÂÂÂÂÂÂgoto end_search; _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |