[Xen-devel] [PATCH] libxl: Be more careful with error handling in libxl__dm_runas_helper()

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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.

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. 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;
-- 
1.8.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel