ptuser: avoid implementation-defined behavior in CreateIdList()

CreateIdList() is an internal subroutine of pr_IDListExpandedMembers(),
used to flatten a hash table of protection IDs into an array that can
be passed to pr_IdToName().  If for some reason the hash table had no
entries, it would call malloc(0) and, depending on how the
the implementation defines this, either return a PRNOMEM error (wrong!)
or else allocate a minimum-sized buffer which pr_IdListExpandedMembers
would then promptly leak.  Compromise between the two behaviors by
not allocating any memory in this case but returning success, and in
the caller check for an empty list and avoid the pointless RPC to
translate no IDs into no names.  pr_IDListExpandedMembers() will return
success, as it previously did in the non-PRNOMEM case.

Change-Id: I8a042bde3e98f5cf248358f37f2e875d6b5b298d
Reviewed-on: http://gerrit.openafs.org/7863
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Simon Wilkinson <simonxwilkinson@gmail.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

diff --git a/src/ptserver/ptuser.c b/src/ptserver/ptuser.c
index b482f76..33e4e4a 100644 (file)
--- a/src/ptserver/ptuser.c
+++ b/src/ptserver/ptuser.c
@@ -147,6 +147,11 @@ CreateIdList(struct idhash *idhash, idlist * alist, afs_int32 select)
     if (select & PRUSERS) {
        entries += idhash->userEntries;
     }
+    if (entries == 0) {
+       alist->idlist_len = 0;
+       alist->idlist_val = NULL;
+       return 0;
+    }
 
     alist->idlist_len = entries;
     alist->idlist_val = malloc(sizeof(afs_int32) * entries);
@@ -759,10 +764,14 @@ pr_IDListExpandedMembers(afs_int32 aid, namelist * lnames)
     code = CreateIdList(members, &lids, (aid < 0 ? PRUSERS : PRGROUPS));
     if (code) {
        goto done;
+    } else if (lids.idlist_len == 0) {
+       /* Avoid the RPC when there's nothing to look up. */
+       lnames->namelist_len = 0;
+       lnames->namelist_val = NULL;
+       goto done;
     }
     code = pr_IdToName(&lids, lnames);
-    if (lids.idlist_len)
-       free(lids.idlist_val);
+    free(lids.idlist_val);
 
   done:
     if (stack)