linux-kernel - Re: [PATCH] selinux: fixed parse warning Using plain integer as NULL pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFqZXNsDrcfMsCwjqv=9H3LswS1jyigKgRCDmER59VJOVd-Edw@mail.gmail.com>
Date:   Sun, 24 Mar 2019 11:57:27 +0100
From:   Ondrej Mosnacek <omosnace@...hat.com>
To:     Hariprasad Kelam <hariprasad.kelam@...il.com>
Cc:     Paul Moore <paul@...l-moore.com>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        Eric Paris <eparis@...isplace.org>,
        Kees Cook <keescook@...omium.org>,
        Richard Guy Briggs <rgb@...hat.com>, jannh@...gle.com,
        Alexey Dobriyan <adobriyan@...il.com>, sgrover@...eaurora.org,
        peter.enderborg@...y.com,
        Kent Overstreet <kent.overstreet@...il.com>,
        selinux@...r.kernel.org,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] selinux: fixed parse warning Using plain integer as NULL pointer

On Sun, Mar 24, 2019 at 11:10 AM Hariprasad Kelam
<hariprasad.kelam@...il.com> wrote:
> Changed  0 --> NULL to avoid sparse warning
>
> Sparse warning below:
>
> sudo make C=2 CF=-D__CHECK_ENDIAN__ M=security
>
>  CHECK   security/selinux/ss/services.c
> security/selinux/ss/services.c:1323:32: warning: Using plain integer as
> NULL pointer
>
> Signed-off-by: Hariprasad Kelam <hariprasad.kelam@...il.com>
> ---
>  security/selinux/ss/services.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index ec62918..30cea59 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1320,7 +1320,7 @@ static int security_sid_to_context_core(struct selinux_state *state,
>         }
>         if (only_invalid && !context->len) {
>                 scontext = NULL;
> -               scontext_len = 0;
> +               scontext_len = NULL;
>                 rc = 0;

The warning is correct, but this patch is the wrong fix. I intended to
set the values to the objects pointed to by the pointers, not the
pointers themselves. As is, the assignments have no effect, because
they only modify the local variables, which are not used further in
the function. Fortunately, *scontext and *scontext_len are already set
to NULL/0 at the beginning of the function, so the code is technically
correct even without them.

The correct fix is actually to remove the assignments entirely. I'll
send a patch that does that tomorrow.

>         } else {
>                 rc = context_struct_to_string(policydb, context, scontext,
> --
> 2.7.4
>

Anyway, thank you for catching the mistake! One more reminder for me
to finally install sparse and add C=1 to my kernel build command
line...


--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.