Re: [PATCH 1/2] chcon: use security_check_context() for context validation

[Pádraig Brady]

On 06/30/2014 05:05 PM, Pádraig Brady wrote:
> On 06/30/2014 04:42 PM, Pádraig Brady wrote:
>> On 06/30/2014 04:12 PM, Namhyung Kim wrote:
>>> It seems context_new() and _free() are used for checking validity of
>>> a specified context.  The libselinux provides security_check_context
>>> for this purpose so use it.
>>>
>>> Note that context_new() can fail for a valid context - e.g. ENOMEM.
>>>
>>> * src/chcon.c (main): Use security_check_context().
>>> ---
>>>  src/chcon.c | 5 +----
>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/src/chcon.c b/src/chcon.c
>>> index 32d4b0f..cd5fba3 100644
>>> --- a/src/chcon.c
>>> +++ b/src/chcon.c
>>> @@ -555,13 +555,10 @@ main (int argc, char **argv)
>>>      }
>>>    else
>>>      {
>>> -      context_t context;
>>>        specified_context = argv[optind++];
>>> -      context = context_new (specified_context);
>>> -      if (!context)
>>> +      if (security_check_context (specified_context) < 0)
>>>          error (EXIT_FAILURE, 0, _("invalid context: %s"),
>>>                 quotearg_colon (specified_context));
>>> -      context_free (context);
>>>      }
>>>  
>>>    if (reference_file && component_specified)
>>>
>>
>> security_check_context() is already used by `runcon`,
>> so this change looks good to me.
> 
> Note the existing use of security_check_context() reports errno,
> and looking at the implementation shows this is set appropriately.
> So I'll change the error() call to use errno before committing.

One further change I've merged is to avoid a
false "possibly unitialised" warning from gcc.

  -  context_t context;
  +  context_t context IF_LINT (= NULL);

There are also new const correctness warnings in the setfileconat() calls.
This is a general awkwardness of the libselinux security_context_t type.
Rather than make our code less const correct, I'll apply the attached
patch later to make it more const correct, and avoid the awkward
security_context_t which is the direction taken by libselinux since:
https://github.com/SELinuxProject/selinux/commit/9eb9c932

thanks,
Pádraig.

libselinux-const-warnings.patch
Description: Text Data