krb5 commit: Allow modifications of empty profiles

ghudson at mit.edu ghudson at mit.edu
Wed Apr 17 18:26:07 EDT 2024


https://github.com/krb5/krb5/commit/fc54edd1dc047aedb211beaa544c5e000fbdb7a6
commit fc54edd1dc047aedb211beaa544c5e000fbdb7a6
Author: Greg Hudson <ghudson at mit.edu>
Date:   Sun Mar 31 12:30:18 2024 -0400

    Allow modifications of empty profiles
    
    Add the notion of a memory-only prf_data_t object, indicated by an
    empty filespec field and appropriate flags (do not reload, always
    dirty, not part of shared trees).  Do nothing when flushing a
    memory-only data object to its backing file.  When setting up an empty
    profile for read/write access, create a memory-only data object
    instead of crashing.
    
    Move prf_data_t mutex initialization into profile_make_prf_data(),
    simplifying its callers.
    
    ticket: 9110

 src/util/profile/prof_file.c | 46 +++++++++++++++++++++++++++++++++++++-------
 src/util/profile/prof_int.h  |  2 ++
 src/util/profile/prof_set.c  | 33 ++++++++++++++-----------------
 src/util/profile/t_profile.c | 28 +++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 26 deletions(-)

diff --git a/src/util/profile/prof_file.c b/src/util/profile/prof_file.c
index aa951df05..b5eddc0d9 100644
--- a/src/util/profile/prof_file.c
+++ b/src/util/profile/prof_file.c
@@ -159,6 +159,10 @@ profile_make_prf_data(const char *filename)
     d->root = NULL;
     d->next = NULL;
     d->fslen = flen;
+    if (k5_mutex_init(&d->lock) != 0) {
+        free(d);
+        return NULL;
+    }
     return d;
 }
 
@@ -239,13 +243,6 @@ errcode_t profile_open_file(const_profile_filespec_t filespec,
     free(expanded_filename);
     prf->data = data;
 
-    retval = k5_mutex_init(&data->lock);
-    if (retval) {
-        free(data);
-        free(prf);
-        return retval;
-    }
-
     retval = profile_update_file(prf, ret_modspec);
     if (retval) {
         profile_close_file(prf);
@@ -262,6 +259,37 @@ errcode_t profile_open_file(const_profile_filespec_t filespec,
     return 0;
 }
 
+prf_file_t profile_open_memory(void)
+{
+    struct profile_node *root = NULL;
+    prf_file_t file = NULL;
+    prf_data_t data;
+
+    file = calloc(1, sizeof(*file));
+    if (file == NULL)
+        goto errout;
+    file->magic = PROF_MAGIC_FILE;
+
+    if (profile_create_node("(root)", NULL, &root) != 0)
+        goto errout;
+
+    data = profile_make_prf_data("");
+    if (data == NULL)
+        goto errout;
+
+    data->root = root;
+    data->flags = PROFILE_FILE_NO_RELOAD | PROFILE_FILE_DIRTY;
+    file->data = data;
+    file->next = NULL;
+    return file;
+
+errout:
+    free(file);
+    if (root != NULL)
+        profile_free_node(root);
+    return NULL;
+}
+
 errcode_t profile_update_file_data_locked(prf_data_t data, char **ret_modspec)
 {
     errcode_t retval;
@@ -468,6 +496,10 @@ errcode_t profile_flush_file_data(prf_data_t data)
     if (!data || data->magic != PROF_MAGIC_FILE_DATA)
         return PROF_MAGIC_FILE_DATA;
 
+    /* Do nothing if this data object has no backing file. */
+    if (*data->filespec == '\0')
+        return 0;
+
     k5_mutex_lock(&data->lock);
 
     if ((data->flags & PROFILE_FILE_DIRTY) == 0) {
diff --git a/src/util/profile/prof_int.h b/src/util/profile/prof_int.h
index 1ee9a8ca1..21c535a5c 100644
--- a/src/util/profile/prof_int.h
+++ b/src/util/profile/prof_int.h
@@ -214,6 +214,8 @@ errcode_t profile_open_file
 	(const_profile_filespec_t file, prf_file_t *ret_prof,
 	 char **ret_modspec);
 
+prf_file_t profile_open_memory(void);
+
 #define profile_update_file(P, M) profile_update_file_data((P)->data, M)
 errcode_t profile_update_file_data
 	(prf_data_t profile, char **ret_modspec);
diff --git a/src/util/profile/prof_set.c b/src/util/profile/prof_set.c
index af4b2f853..aeea676cb 100644
--- a/src/util/profile/prof_set.c
+++ b/src/util/profile/prof_set.c
@@ -24,7 +24,7 @@
 static errcode_t rw_setup(profile_t profile)
 {
     prf_file_t      file;
-    errcode_t       retval = 0;
+    prf_data_t      new_data;
 
     if (!profile)
         return PROF_NO_PROFILE;
@@ -32,6 +32,12 @@ static errcode_t rw_setup(profile_t profile)
     if (profile->magic != PROF_MAGIC_PROFILE)
         return PROF_MAGIC_PROFILE;
 
+    /* If the profile has no files, create a memory-only data object. */
+    if (profile->first_file == NULL) {
+        profile->first_file = profile_open_memory();
+        return (profile->first_file == NULL) ? ENOMEM : 0;
+    }
+
     file = profile->first_file;
 
     profile_lock_global();
@@ -43,33 +49,22 @@ static errcode_t rw_setup(profile_t profile)
     }
 
     if ((file->data->flags & PROFILE_FILE_SHARED) != 0) {
-        prf_data_t new_data;
         new_data = profile_make_prf_data(file->data->filespec);
         if (new_data == NULL) {
-            retval = ENOMEM;
-        } else {
-            retval = k5_mutex_init(&new_data->lock);
-            if (retval == 0) {
-                new_data->root = NULL;
-                new_data->flags = file->data->flags & ~PROFILE_FILE_SHARED;
-                new_data->timestamp = 0;
-                new_data->upd_serial = file->data->upd_serial;
-            }
-        }
-
-        if (retval != 0) {
             profile_unlock_global();
-            free(new_data);
-            return retval;
+            return ENOMEM;
         }
+        new_data->root = NULL;
+        new_data->flags = file->data->flags & ~PROFILE_FILE_SHARED;
+        new_data->timestamp = 0;
+        new_data->upd_serial = file->data->upd_serial;
+
         profile_dereference_data_locked(file->data);
         file->data = new_data;
     }
 
     profile_unlock_global();
-    retval = profile_update_file(file, NULL);
-
-    return retval;
+    return profile_update_file(file, NULL);
 }
 
 
diff --git a/src/util/profile/t_profile.c b/src/util/profile/t_profile.c
index bffd11561..0e859b97c 100644
--- a/src/util/profile/t_profile.c
+++ b/src/util/profile/t_profile.c
@@ -373,6 +373,33 @@ test_merge_subsections(void)
     profile_release(p);
 }
 
+/* Regression test for #9110 (null dereference when modifying an empty
+ * profile), and various other operations on an initially empty profile. */
+static void
+test_empty(void)
+{
+    profile_t p;
+    const char *n1[] = { "section", NULL };
+    const char *n2[] = { "section", "var", NULL };
+    char **values;
+
+    check(profile_init(NULL, &p));
+    check(profile_add_relation(p, n1, NULL));
+    check(profile_add_relation(p, n2, "value"));
+    check(profile_flush(p));    /* should succeed but do nothing */
+    check(profile_get_values(p, n2, &values));
+    assert(strcmp(values[0], "value") == 0 && values[1] == NULL);
+    profile_free_list(values);
+    profile_flush_to_file(p, "test3.ini");
+    profile_release(p);
+
+    profile_init_path("test3.ini", &p);
+    check(profile_get_values(p, n2, &values));
+    assert(strcmp(values[0], "value") == 0 && values[1] == NULL);
+    profile_free_list(values);
+    profile_release(p);
+}
+
 int
 main(void)
 {
@@ -386,4 +413,5 @@ main(void)
     test_delete_ordering();
     test_flush_to_file();
     test_merge_subsections();
+    test_empty();
 }


More information about the cvs-krb5 mailing list