[SSSD] [PATCH] IPA: Fix SELinux mapping order memory hierarchy
Sumit Bose
sbose at redhat.com
Thu Apr 3 07:45:46 UTC 2014
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: lslebodn at 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 <jhrozek at 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 at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
More information about the sssd-devel
mailing list