2019-03-24 10:11:49

by Hariprasad Kelam

Subject: [PATCH] selinux: fixed parse warning Using plain integer as NULL pointer

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 <[email protected]>
---
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;
} else {
rc = context_struct_to_string(policydb, context, scontext,
--
2.7.4

2019-03-24 10:58:31

by Ondrej Mosnacek

Subject: Re: [PATCH] selinux: fixed parse warning Using plain integer as NULL pointer

On Sun, Mar 24, 2019 at 11:10 AM Hariprasad Kelam
<[email protected]> 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 <[email protected]>
> ---
> 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.