On Wed, Dec 20, 2017 at 10:47:38AM -0500, Justin Stephenson wrote:
> Hello,
> 
> I have been working on this ticket[1] to allow selection of file paths for
> alternate passwd and group files to be used with the files provider.
> 
> First, just to make sure I am not completely on the wrong track I would like
> to ask for someone to take a quick look at the current work-in-progress code
> I have here in my SSSD fork to validate that it matches up with the expected
> implementation.
> 
> https://github.com/SSSD/sssd/compare/master...justin-stephenson:jstephen-files-multi-sources
> 
> Second, assuming I am not completely off track I would appreciate any help
> on a code problem I am having here when sf_init() gets called to setup
> watches on the passwd_files and group_files lists, the list values are
> corrupted or inaccessible. I started off with the code from the function
> files_init_alt_files() inside sssm_files_init() directly and everything
> works as expected, the problem started when I moved the code into this
> separate function.
> 
> The list values are retrieved from the new alt_passwd_files and
> alt_group_files options or default to /etc/passwd and /etc/group files.
> Inside sssm_files_init() I am able to access the correct values of each
> string element in the list but when sf_init() is called the list is corrupt
> or I am performing some undefined behavior that I cannot determine.

Hi Justin,

I'm sorry I forgot about this issue even though you pinged me about it
via an e-mail.

I think the following diff might help:
diff --git a/src/providers/files/files_init.c b/src/providers/files/files_init.c
index e0064bc4b..7fd0ff0ca 100644
--- a/src/providers/files/files_init.c
+++ b/src/providers/files/files_init.c
@@ -77,7 +77,7 @@ static errno_t files_init_alt_files(TALLOC_CTX *mem_ctx,
             goto done;
         }
 
-        passwd_files[0] = talloc_strdup(tmp_ctx, DEFAULT_PASSWD_FILE);
+        passwd_files[0] = talloc_strdup(passwd_files, DEFAULT_PASSWD_FILE);
         if (passwd_files[0] == NULL) {
             ret = ENOMEM;
             goto done;
@@ -101,7 +101,7 @@ static errno_t files_init_alt_files(TALLOC_CTX *mem_ctx,
         }
 
         for (i = 0; i < num_passwd_files; i++) {
-            passwd_files[i] = talloc_strdup(tmp_ctx, passwd_list[i]);
+            passwd_files[i] = talloc_strdup(passwd_files, passwd_list[i]);
                 if (passwd_files[i] == NULL) {
                     ret = ENOMEM;
                     goto done;
@@ -120,7 +120,7 @@ static errno_t files_init_alt_files(TALLOC_CTX *mem_ctx,
             goto done;
         }
 
-        group_files[0] = talloc_strdup(tmp_ctx, DEFAULT_GROUP_FILE);
+        group_files[0] = talloc_strdup(group_files, DEFAULT_GROUP_FILE);
         if (group_files[0] == NULL) {
             ret = ENOMEM;
             goto done;
@@ -144,7 +144,7 @@ static errno_t files_init_alt_files(TALLOC_CTX *mem_ctx,
         }
 
         for (i = 0; i < num_group_files; i++) {
-            group_files[i] = talloc_strdup(tmp_ctx, group_list[i]);
+            group_files[i] = talloc_strdup(group_files, group_list[i]);
                 if (group_files[i] == NULL) {
                     ret = ENOMEM;
                     goto done;

Without the diff, the memory hierarchy looks like this:
        tmp_ctx +
                + group_files
                + altfile1
                + altfile2
                + ...

So at the end of the function, when you steal group_files, you only steal
the array "envelope", but not the contents. With the fix, the hierarchy
looks like this:
        tmp_ctx +
                + group_files +
                              + altfile1
                              + altfile2
                              + ...

So when you steal group_files, you also steal what group_files owns.
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to