setcifsacl: fix some bugs in build_fetched_aces

Pavel Raiskup reported the following defects that he found with Coverity:

"If the variable 'facesptr' on line cifs-utils-4.8.1/setcifsacl.c|365|
has not enough memory to be allocated, program 'setcifsacl' will fail
with segfault on line 365 (dereferencing facesptr)."

"you may return freed pointer here. There is some kind of return code
('rc') which should be transferred to >NULL< when is rc nonzero (and
returned)"

There are also a couple of other bugs here:

malloc doesn't necessarily set errno to anything when an allocation
fails, so having the error handling rely on that is wrong.

Fix all of these bugs by reorganzing this function to fix up the error
handling.

Reported-by: Pavel Raiskup <praiskup@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>

1 files changed, 12 insertions, 18 deletions

diff --git a/setcifsacl.c b/setcifsacl.c
index ce13638..668ccfb 100644
--- a/setcifsacl.c
+++ b/setcifsacl.c
@@ -347,42 +347,36 @@ get_numfaces(struct cifs_ntsd *pntsd, ssize_t acl_len,
 static struct cifs_ace **
 build_fetched_aces(char *daclptr, int numfaces)
 {
-	int i, j, rc = 0, acl_size;
+	int i, acl_size;
 	char *acl_base;
 	struct cifs_ace *pace, **facesptr;
 
-	facesptr = (struct cifs_ace **)malloc(numfaces *
-					sizeof(struct cifs_aces *));
+	facesptr = calloc(numfaces, sizeof(struct cifs_aces *));
 	if (!facesptr) {
 		printf("%s: Error %d allocating ACE array",
 				__func__, errno);
-		rc = errno;
+		return facesptr;
 	}
 
 	acl_base = daclptr;
 	acl_size = sizeof(struct cifs_ctrl_acl);
 	for (i = 0; i < numfaces; ++i) {
 		facesptr[i] = malloc(sizeof(struct cifs_ace));
-		if (!facesptr[i]) {
-			rc = errno;
-			goto build_fetched_aces_ret;
-		}
+		if (!facesptr[i])
+			goto build_fetched_aces_err;
 		pace = (struct cifs_ace *) (acl_base + acl_size);
 		memcpy(facesptr[i], pace, sizeof(struct cifs_ace));
 		acl_base = (char *)pace;
 		acl_size = le16toh(pace->size);
 	}
-
-build_fetched_aces_ret:
-	if (rc) {
-		printf("%s: Invalid fetched ace\n", __func__);
-		if (i) {
-			for (j = i; j >= 0; --j)
-				free(facesptr[j]);
-		}
-		free(facesptr);
-	}
 	return facesptr;
+
+build_fetched_aces_err:
+	printf("%s: Invalid fetched ace\n", __func__);
+	for (i = 0; i < numfaces; ++i)
+		free(facesptr[i]);
+	free(facesptr);
+	return NULL;
 }
 
 static int