Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190324101003.GA17131@hari-Inspiron-1545>
In-Reply-To: <20190324101003.GA17131@hari-Inspiron-1545>
From:   Ondrej Mosnacek <omosnace@redhat.com>
Date:   Sun, 24 Mar 2019 11:57:27 +0100
Message-ID: <CAFqZXNsDrcfMsCwjqv=9H3LswS1jyigKgRCDmER59VJOVd-Edw@mail.gmail.com>
Subject: Re: [PATCH] selinux: fixed parse warning Using plain integer as NULL pointer
To:     Hariprasad Kelam <hariprasad.kelam@gmail.com>
Cc:     Paul Moore <paul@paul-moore.com>,
        Stephen Smalley <sds@tycho.nsa.gov>,
        Eric Paris <eparis@parisplace.org>,
        Kees Cook <keescook@chromium.org>,
        Richard Guy Briggs <rgb@redhat.com>, jannh@google.com,
        Alexey Dobriyan <adobriyan@gmail.com>, sgrover@codeaurora.org,
        peter.enderborg@sony.com,
        Kent Overstreet <kent.overstreet@gmail.com>,
        selinux@vger.kernel.org,
        Linux kernel mailing list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Mar 24, 2019 at 11:10 AM Hariprasad Kelam
<hariprasad.kelam@gmail.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@gmail.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.