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@xxxxxxxxxx>
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxx>
---
setcifsacl.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/setcifsacl.c b/setcifsacl.c
index 5016264..29c83ba 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
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
