krb5 commit: Fix profile_flush_to_file() state corruption

Greg Hudson ghudson at mit.edu
Fri Jul 1 16:18:30 EDT 2016


https://github.com/krb5/krb5/commit/32a05995ff9df0d5ef8aff0d020900a37747670d
commit 32a05995ff9df0d5ef8aff0d020900a37747670d
Author: Greg Hudson <ghudson at mit.edu>
Date:   Thu Jun 23 12:01:56 2016 -0400

    Fix profile_flush_to_file() state corruption
    
    In write_data_to_file(), do not clear the profile data object's flags.
    If the call to this function resulted from profile_flush_to_file(), we
    do not want to clear the DIRTY flag, and we especially do not want to
    clear the SHARED flag for a data object which is part of
    g_shared_trees.  Instead, clear the DIRTY flag in
    profile_flush_file_data().
    
    Add a test case to prof_test1 to exercise the bug in unfixed code.
    Also modify test1 to abandon the altered profile after flushing it to
    a file, to preserve the external behavior of the script before this
    fix.
    
    ticket: 8431
    target_version: 1.14-next
    tags: pullup

 src/util/profile/prof_file.c |    2 +-
 src/util/profile/prof_test1  |   22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/util/profile/prof_file.c b/src/util/profile/prof_file.c
index d774593..907c119 100644
--- a/src/util/profile/prof_file.c
+++ b/src/util/profile/prof_file.c
@@ -473,7 +473,6 @@ static errcode_t write_data_to_file(prf_data_t data, const char *outfile,
         }
     }
 
-    data->flags = 0;
     retval = 0;
 
 errout:
@@ -509,6 +508,7 @@ errcode_t profile_flush_file_data(prf_data_t data)
     }
 
     retval = write_data_to_file(data, data->filespec, 0);
+    data->flags &= ~PROFILE_FILE_DIRTY;
     k5_mutex_unlock(&data->lock);
     return retval;
 }
diff --git a/src/util/profile/prof_test1 b/src/util/profile/prof_test1
index d0bb187..984eb3b 100644
--- a/src/util/profile/prof_test1
+++ b/src/util/profile/prof_test1
@@ -40,7 +40,7 @@ proc test1 {} {
     #profile_iterator_free $iter
     catch {file delete $wd/test3.ini}
     profile_flush_to_file $p $wd/test3.ini
-    profile_release $p
+    profile_abandon $p
 
     if $verbose { puts "Reloading new profile" }
     set p [profile_init_path $wd/test3.ini]
@@ -318,6 +318,25 @@ proc test8 {} {
     puts "OK: test8: relation order in the presence of deletions"
 }
 
+proc test9 {} {
+    global wd verbose
+
+    # Regression test for #8431: profile_flush_to_file erroneously
+    # cleared the DIRTY and SHARED flags from the data object, which
+    # could lead to a dangling reference in g_shared_trees on release.
+    set p [profile_init_path $wd/test2.ini]
+    catch {file delete $wd/test3.ini}
+    profile_flush_to_file $p $wd/test3.ini
+    profile_release $p
+
+    # If a dangling reference was created in g_shared_trees, the next
+    # profile open will trigger an assertion failure.
+    set p [profile_init_path $wd/test2.ini]
+    profile_release $p
+
+    puts "OK: test9: profile_flush_to_file with no changes"
+}
+
 test1
 test2
 test3
@@ -326,5 +345,6 @@ test5
 test6
 test7
 test8
+test9
 
 exit 0


More information about the cvs-krb5 mailing list