[SSSD] [PATCH] Passing NULL as map_order in create_order_array is invalid.

Lukas Slebodnik lslebodn at redhat.com
Fri Apr 4 17:26:22 UTC 2014


On (04/04/14 17:08), Michal Židek wrote:
>Reported by Clang. See patch description for more info.
>
>Patch is in attachment.
>
>Thanks,
>Michal

>From a0c94bc4c5df2955e82a7dc0852a86019286577e Mon Sep 17 00:00:00 2001
>From: Michal Zidek <mzidek at redhat.com>
>Date: Fri, 4 Apr 2014 16:53:48 +0200
>Subject: [PATCH] Passing NULL as map_order in create_order_array is invalid.
>
>Reported by Clang. This is actually false positive because
>a NULL pointer in map_order would cause the function above
>(sysdb_store_selinux_config) to fail and create_order_array
>would be skipped. So this patch is another line of
>defense that just makes the code more readable and silences
>the message from Clang.
>---
> src/providers/ipa/ipa_selinux.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
>index e0d7a00..5ee1e74 100644
>--- a/src/providers/ipa/ipa_selinux.c
>+++ b/src/providers/ipa/ipa_selinux.c
>@@ -595,6 +595,10 @@ static errno_t create_order_array(TALLOC_CTX *mem_ctx, const char *map_order,
>     int len;
>     size_t order_count;
> 
>+    if (map_order == NULL) {
>+        return EINVAL;
>+    }
>+
>     tmp_ctx = talloc_new(NULL);
>     if (tmp_ctx == NULL) {
>         ret = ENOMEM;
>-- 
>1.7.11.2
>
This patch fixed warning from clang static analyser.

But as you wrote, it is false positive because sssd would crash few lines
before calling create_order_array in function sysdb_store_selinux_config.

Why I think it is false positive:
    -because map_order cannot be null.

static errno_t
ipa_get_selinux_recv(struct tevent_req *req,

        //snip

   if (state->defaults != NULL) {

        //snip

        *map_order = talloc_strdup(mem_ctx, tmp_str);
        if (*map_order == NULL) {
            talloc_zfree(*default_user);
            return ENOMEM;
        }
    } else {
        *map_order = NULL;
        *default_user = NULL;
    }
    ^^^^^
else branch is dead code.


state->defaults could be NULL in offline mode, but only in the first version
of file src/providers/ipa/ipa_selinux.c The behaviour was rewritten in commit

commit a004ce714c20a7a5324393ea47f5dc115eb20713
Author: Jakub Hrozek <jhrozek at redhat.com>

    SELINUX: Process maps even when offline


We can ignore this warning and function ipa_get_selinux_recv should be
refactored.

LS



More information about the sssd-devel mailing list