Re: [SSSD] [PATCH] IPA: Fix SELinux mapping order memory hierarchy
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
Re: [SSSD] [PATCH] IPA: Fix SELinux mapping order memory hierarchy
On Thu, Apr 03, 2014 at 09:45:46AM +0200, Sumit Bose wrote:
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
Would you accept this small patch for 1.11 and your proposed (and
better, I agree) change for master?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] IPA: Fix SELinux mapping order memory hierarchy
On Thu, Apr 03, 2014 at 12:58:03PM +0200, Jakub Hrozek wrote:
On Thu, Apr 03, 2014 at 09:45:46AM +0200, Sumit Bose wrote:
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
Would you accept this small patch for 1.11 and your proposed (and
better, I agree) change for master?
sure, ACK.
But I think it would be better to apply this one to 1.11 and master and
then add another patch with the changes to master.
bye,
Sumit
___
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
Re: [SSSD] [PATCH] IPA: Fix SELinux mapping order memory hierarchy
On Thu, Apr 03, 2014 at 01:25:04PM +0200, Sumit Bose wrote:
On Thu, Apr 03, 2014 at 12:58:03PM +0200, Jakub Hrozek wrote:
On Thu, Apr 03, 2014 at 09:45:46AM +0200, Sumit Bose wrote:
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
Would you accept this small patch for 1.11 and your proposed (and
better, I agree) change for master?
sure, ACK.
But I think it would be better to apply this one to 1.11 and master and
then add another patch with the changes to master.
bye,
Sumit
OK, thank you for the acks. I filed another ticket for the refactor and
gave it to Michal:
https://fedorahosted.org/sssd/ticket/2304
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] IPA: Fix SELinux mapping order memory hierarchy
On Thu, Apr 03, 2014 at 05:13:19PM +0200, Jakub Hrozek wrote:
On Thu, Apr 03, 2014 at 01:25:04PM +0200, Sumit Bose wrote:
On Thu, Apr 03, 2014 at 12:58:03PM +0200, Jakub Hrozek wrote:
On Thu, Apr 03, 2014 at 09:45:46AM +0200, Sumit Bose wrote:
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
Would you accept this small patch for 1.11 and your proposed (and
better, I agree) change for master?
sure, ACK.
But I think it would be better to apply this one to 1.11 and master and
then add another patch with the changes to master.
bye,
Sumit
OK, thank you for the acks. I filed another ticket for the refactor and
gave it to Michal:
https://fedorahosted.org/sssd/ticket/2304
Pushed upstream:
master: 355b8a655cfcc4e783077d12f76b55da1d23fb87
sssd-1-11: ac93a2d27415abd730aa1063b1689def8be9dbe9
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
