On Wed, Apr 02, 2014 at 10:29:15PM +0200, Jakub Hrozek wrote: > Hi, > > please see the attached patch that fixes a use-after-free bug in the > SELinux provider. > > Pair-Programmed-With: lsleb...@redhat.com
The patch solves the issue, but I think the issue was there in the first place, because the 'order' string is somewhat hidden. I would suggest to use a struct like struct order_ctx { const char *order; char **order_array; size_t order_count; }; It might be a bit overkill for this small usage, but I think it makes the intention and the relation between order and order_array more clear. bye, Sumit > From 538afda9760d0baae716b159e36da3c5edcaef29 Mon Sep 17 00:00:00 2001 > From: Jakub Hrozek <jhro...@redhat.com> > Date: Wed, 2 Apr 2014 22:11:59 +0200 > Subject: [PATCH] IPA: Fix SELinux mapping order memory hierarchy > > https://fedorahosted.org/sssd/ticket/2300 > > The list of SELinux mapping orders was allocated on tmp_ctx and parsed > into an array. The array itself was correctly allocated on mem_ctx but > its contents remained on tmp_ctx, leading to a use-after-free error. > This patch fixes the memory hierarchy so that both the array and its > contents are allocated on mem_ctx. > --- > src/providers/ipa/ipa_selinux.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c > index > 1fc0c68d0bdc7cc5131d61694db26a208c51d300..7c3ce45c606595b52b13beca749c1c9d93bd31f2 > 100644 > --- a/src/providers/ipa/ipa_selinux.c > +++ b/src/providers/ipa/ipa_selinux.c > @@ -601,21 +601,15 @@ static errno_t create_order_array(TALLOC_CTX *mem_ctx, > const char *map_order, > goto done; > } > > - order = talloc_strdup(tmp_ctx, map_order); > - if (order == NULL) { > - ret = ENOMEM; > - goto done; > - } > - len = strlen(order); > - > /* The "order" string contains one or more SELinux user records > * separated by $. Now we need to create an array of string from > * this one string. First find out how many elements in the array > * will be. This way only one alloc will be necessary for the array > */ > order_count = 1; > + len = strlen(map_order); > for (i = 0; i < len; i++) { > - if (order[i] == '$') order_count++; > + if (map_order[i] == '$') order_count++; > } > > order_array = talloc_array(tmp_ctx, char *, order_count); > @@ -624,6 +618,12 @@ static errno_t create_order_array(TALLOC_CTX *mem_ctx, > const char *map_order, > goto done; > } > > + order = talloc_strdup(order_array, map_order); > + if (order == NULL) { > + ret = ENOMEM; > + goto done; > + } > + > /* Now fill the array with pointers to the original string. Also > * use binary zeros to make multiple string out of the one. > */ > -- > 1.9.0 > > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel