On Fri, 2011-11-11 at 13:41 -0500, Rob Crittenden wrote:
> Martin Kosek wrote:
> > This patch fixes 2 coverity issues:
> >   * ipa-client/config.c: CID 11090: Resource leak
> >   * ipa-client/ipa-getkeytab.c: CID 11018: Unchecked return value
> >
> > https://fedorahosted.org/freeipa/ticket/2035
> 
> You don't need to test a variable before you free it, so you can just 
> call free(data).

Right, I was over-defensive here. Fixed.

> 
> Since we are capturing the kerberos error should it be displayed as well 
> in ipa-getkeytab?
> 
> rob
> 

Good idea, it should. Unfortunately, this is not done properly across
all ipa-getkeytab.c. ipa-rmkeytab.c does this much better.

I added error messages at least to the affected function to keep the
patch compact.

Martin
>From 2c94190c14a35d5db7bf302c86b90f2690f9129f Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 8 Nov 2011 17:59:45 +0100
Subject: [PATCH] Fix coverity issues in client CLI tools

This patch fixes 2 coverity issues:
 * ipa-client/config.c: CID 11090: Resource leak
 * ipa-client/ipa-getkeytab.c: CID 11018: Unchecked return value

https://fedorahosted.org/freeipa/ticket/2035
---
 ipa-client/config.c        |   20 ++++++++++++--------
 ipa-client/ipa-getkeytab.c |   14 ++++++++++++--
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/ipa-client/config.c b/ipa-client/config.c
index 493d740207ffca4275f512fad97d40e4f1e8fa05..ecc126ff47cc46aba541f0a9c29f06b40a93680b 100644
--- a/ipa-client/config.c
+++ b/ipa-client/config.c
@@ -45,28 +45,29 @@
 char *
 read_config_file(const char *filename)
 {
-    int fd;
+    int fd = -1;
     struct stat st;
-    char *data, *dest;
+    char *data = NULL;
+    char *dest;
     size_t left;
 
     fd = open(filename, O_RDONLY);
     if (fd == -1) {
         fprintf(stderr, _("cannot open configuration file %s\n"), filename);
-        return NULL;
+        goto error_out;
     }
 
     /* stat() the file so we know the size and can pre-allocate the right
      * amount of memory. */
     if (fstat(fd, &st) == -1) {
         fprintf(stderr, _("cannot stat() configuration file %s\n"), filename);
-        return NULL;
+        goto error_out;
     }
     left = st.st_size;
     data = malloc(st.st_size + 1);
     if (data == NULL) {
         fprintf(stderr, _("out of memory\n"));
-        return NULL;
+        goto error_out;
     }
     dest = data;
     while (left != 0) {
@@ -77,9 +78,7 @@ read_config_file(const char *filename)
             break;
         if (res < 0) {
             fprintf(stderr, _("read error\n"));
-            close(fd);
-            free(dest);
-            return NULL;
+            goto error_out;
         }
         dest += res;
         left -= res;
@@ -87,6 +86,11 @@ read_config_file(const char *filename)
     close(fd);
     *dest = 0;
     return data;
+
+error_out:
+    if (fd != -1) close(fd);
+    free(data);
+    return NULL;
 }
 
 char *
diff --git a/ipa-client/ipa-getkeytab.c b/ipa-client/ipa-getkeytab.c
index 5a521d04127491193e971a00697f3e077796e01e..28ef5b5a578ea74f4af14efd063236dd009f07fe 100644
--- a/ipa-client/ipa-getkeytab.c
+++ b/ipa-client/ipa-getkeytab.c
@@ -82,14 +82,24 @@ static int ldap_sasl_interact(LDAP *ld, unsigned flags, void *priv_data, void *s
 			krberr = krb5_init_context(&krbctx);
 
 			if (krberr) {
-				fprintf(stderr, _("Kerberos context initialization failed\n"));
+				fprintf(stderr, _("Kerberos context initialization failed: %s (%d)\n"),
+                                error_message(krberr), krberr);
 				in->result = NULL;
 				in->len = 0;
 				ret = LDAP_LOCAL_ERROR;
 				break;
 			}
 
-			krb5_unparse_name(krbctx, princ, &outname);
+			krberr = krb5_unparse_name(krbctx, princ, &outname);
+
+			if (krberr) {
+                fprintf(stderr, _("Unable to parse principal: %s (%d)\n"),
+                                error_message(krberr), krberr);
+				in->result = NULL;
+				in->len = 0;
+				ret = LDAP_LOCAL_ERROR;
+				break;
+			}
 
 			in->result = outname;
 			in->len = strlen(outname);
-- 
1.7.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to