Improve code hygiene of kdb5_util dump helpers

kdb5_util dump can very briefly leak a file handle if the ok file
cannot be locked, or if the existing dump file cannot be read.  Add a
cleanup handler to prep_ok_file() and use proper output parameter
handling.  Change current_dump_sno_in_ulog() to close its file handle
before checking the result of fgets().  Reported by Bean Zhang.

diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index 86e046c429bb6ceeeff200befe3b0e298621f1d1..c9574c6e129dd9ea98312a965662a02fb54975d9 100644 (file)
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -181,34 +181,44 @@ finish_ofile(char *ofile, char **tmpname)
 }
 
 /* Create the .dump_ok file. */
-static int
-prep_ok_file(krb5_context context, char *file_name, int *fd)
+static krb5_boolean
+prep_ok_file(krb5_context context, char *file_name, int *fd_out)
 {
     static char ok[] = ".dump_ok";
     krb5_error_code retval;
-    char *file_ok;
+    char *file_ok = NULL;
+    int fd = -1;
+    krb5_boolean success = FALSE;
+
+    *fd_out = -1;
 
     if (asprintf(&file_ok, "%s%s", file_name, ok) < 0) {
         com_err(progname, ENOMEM, _("while allocating dump_ok filename"));
-        exit_status++;
-        return 0;
+        goto cleanup;
     }
 
-    *fd = open(file_ok, O_WRONLY | O_CREAT | O_TRUNC, 0600);
-    if (*fd == -1) {
+    fd = open(file_ok, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+    if (fd == -1) {
         com_err(progname, errno, _("while creating 'ok' file, '%s'"), file_ok);
-        exit_status++;
-        free(file_ok);
-        return 0;
+        goto cleanup;
     }
-    retval = krb5_lock_file(context, *fd, KRB5_LOCKMODE_EXCLUSIVE);
+    retval = krb5_lock_file(context, fd, KRB5_LOCKMODE_EXCLUSIVE);
     if (retval) {
         com_err(progname, retval, _("while locking 'ok' file, '%s'"), file_ok);
-        free(file_ok);
-        return 0;
+        goto cleanup;
     }
+
+    *fd_out = fd;
+    fd = -1;
+    success = TRUE;
+
+cleanup:
     free(file_ok);
-    return 1;
+    if (fd != -1)
+        close(fd);
+    if (!success)
+        exit_status++;
+    return success;
 }
 
 /*
@@ -1227,16 +1237,17 @@ current_dump_sno_in_ulog(krb5_context context, const char *ifile)
     update_status_t status;
     dump_version *junk;
     kdb_last_t last;
-    char buf[BUFSIZ];
+    char buf[BUFSIZ], *r;
     FILE *f;
 
     f = fopen(ifile, "r");
     if (f == NULL)
         return 0;              /* aliasing other errors to ENOENT here is OK */
 
-    if (fgets(buf, sizeof(buf), f) == NULL)
-        return errno ? -1 : 0;
+    r = fgets(buf, sizeof(buf), f);
     fclose(f);
+    if (r == NULL)
+        return errno ? -1 : 0;
 
     if (!parse_iprop_header(buf, &junk, &last))
         return 0;