Fixing external/checkpolicy issues found by Klocwork

Fixing external/checkpolicy issues found by Klocwork

[Alice Chu]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

Hello,

Attached you will find the Klocwork report on seandroid master branch external/checkpolicy. The following is the fix for issues found in policy_define.c.
Please review and give me your feedback.

Thank you very much,
Alice Chu

============================================================================

Re: Fixing external/checkpolicy issues found by Klocwork

[Stephen Smalley]

On 01/07/2013 08:29 PM, Alice Chu wrote:
> Hello,
>
> Attached you will find the Klocwork report on seandroid master branch external/checkpolicy. The following is the fix for issues found in policy_define.c.
> Please review and give me your feedback.
>
> Thank you very much,
> Alice Chu
>
> ============================================================================
>>From 18555451c5831fd95044e665d3dc514eb69e3b75 Mon Sep 17 00:00:00 2001
> From: Alice Chu <alice.chu@sta.samsung.com>
> Date: Mon, 7 Jan 2013 15:29:29 -0800
> Subject: [PATCH] Fix issues found by Klocwork
>
> Change-Id: Ic3a01364b6855529f6b58a8820c6011a22c21841
> ---
>   policy_define.c |   24 +++++++++++++++++++-----
>   1 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/policy_define.c b/policy_define.c
> index 2c12447..504af69 100644
> --- a/policy_define.c
> +++ b/policy_define.c
> @@ -3583,6 +3591,11 @@ static int parse_security_context(context_struct_t * c)
>   	}
>
>   	context_init(c);
> +	/* check context c to make sure ok to dereference c later */
> +	if (c == NULL) {
> +		yyerror("null context pointer!");
> +		goto bad;
> +	}
>
>   	/* extract the user */
>   	id = queue_remove(id_queue);

I think you want this check before context_init(), as it dereferences c.
And then just return -1 in the error path.
This btw is an illegal state as NULL should only be passed if pass == 1.




--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Fixing external/checkpolicy issues found by Klocwork

[Eric Paris]

I've recently done a bunch of work to handle errors found by coverity.
 I'll take a look at these and see which still make sense.  Thanks!

On Tue, Jan 8, 2013 at 10:07 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 01/07/2013 08:29 PM, Alice Chu wrote:
>>
>> Hello,
>>
>> Attached you will find the Klocwork report on seandroid master branch
>> external/checkpolicy. The following is the fix for issues found in
>> policy_define.c.
>> Please review and give me your feedback.
>>
>> Thank you very much,
>> Alice Chu
>>
>>
>> ============================================================================
>>>
>>> From 18555451c5831fd95044e665d3dc514eb69e3b75 Mon Sep 17 00:00:00 2001
>>
>> From: Alice Chu <alice.chu@sta.samsung.com>
>> Date: Mon, 7 Jan 2013 15:29:29 -0800
>> Subject: [PATCH] Fix issues found by Klocwork
>>
>> Change-Id: Ic3a01364b6855529f6b58a8820c6011a22c21841
>> ---
>>   policy_define.c |   24 +++++++++++++++++++-----
>>   1 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/policy_define.c b/policy_define.c
>> index 2c12447..504af69 100644
>> --- a/policy_define.c
>> +++ b/policy_define.c
>> @@ -3583,6 +3591,11 @@ static int parse_security_context(context_struct_t
>> * c)
>>         }
>>
>>         context_init(c);
>> +       /* check context c to make sure ok to dereference c later */
>> +       if (c == NULL) {
>> +               yyerror("null context pointer!");
>> +               goto bad;
>> +       }
>>
>>         /* extract the user */
>>         id = queue_remove(id_queue);
>
>
> I think you want this check before context_init(), as it dereferences c.
> And then just return -1 in the error path.
> This btw is an illegal state as NULL should only be passed if pass == 1.
>
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov
> with
> the words "unsubscribe selinux" without quotes as the message.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: Fixing external/checkpolicy issues found by Klocwork

[Alice Chu]

Hi Stephen,

Thanks for pointing out the error. Here is the change (with a more specific commit comment).