Re: [PATCH] ls: with -Z, show SMACK context for each file (v3)

[Pádraig Brady]

On 06/03/2013 06:05 PM, Jarkko Sakkinen wrote:
> Enable showing of file SMACK labels with -Z command-line switch.
> 
> * src/ls.c (gobble_file): Output the smack context if available.
> * src/ls.c: New function getsmackcon_cache() for grabbing SMACK label.

    ls: with -Z, show SMACK context if available

    Enable showing of file SMACK labels with -Z command-line switch.

    * src/ls.c (gobble_file): Output the smack context if available.
    * src/ls.c (getsmackcon_cache): New function for getting SMACK label.

> diff --git a/src/ls.c b/src/ls.c
> @@ -1371,7 +1385,7 @@ main (int argc, char **argv)
>  
>    format_needs_stat = sort_type == sort_time || sort_type == sort_size
>      || format == long_format
> -    || print_scontext
> +    || print_scontext != no_context

again these changes are redundant

> @@ -2801,25 +2819,37 @@ errno_unsupported (int err)
>     Like getfilecon/lgetfilecon, but when F's st_dev says it's on a known-
>     SELinux-challenged file system, fail with ENOTSUP immediately.  */
>  static int
> -getfilecon_cache (char const *file, struct fileinfo *f, bool deref)
> +getfilecon_cache (char const *file, struct fileinfo *f, bool deref,
> +                  enum scontext_type scontext)
>  {
>    /* st_dev of the most recently processed device for which we've
>       found that [l]getfilecon fails indicating lack of support.  */
>    static dev_t unsupported_device;
>  
> -  if (f->stat.st_dev == unsupported_device)
> +  if (f->stat.st_dev == unsupported_device || scontext == no_context)
> +    {
> +      errno = ENOTSUP;
> +      return -1;
> +    }

We need to read the selinux/smack context with -l even without -Z
so the appropriate "extra" perm bits can be displayed appropriately.

> +  int r = 0;
> +  if (scontext == selinux_context)
> +    r = (deref
> +         ? getfilecon (file, &f->scontext)
> +         : lgetfilecon (file, &f->scontext));
> +  else if (scontext == smack_context)
> +    r = smack_new_label_from_path (file, "security.SMACK64", deref,
> +                                   &f->scontext);

This is a new function in libsmack.
Shouldn't you key HAVE_SMACK on its availability in m4/jm-macros.m4 ?
Also I'm a little worried by the mixing of security_context_t*
and char*, specifically that freecon() is called on both.
The _currently_ isn't an issue, but it's awkward and best avoided if possible.

>            if (err == 0)
> -            have_selinux = ! STREQ ("unlabeled", f->scontext);
> +            if (print_scontext == selinux_context)
> +              have_context = ! STREQ ("unlabeled", f->scontext);
> +            else if (print_scontext == smack_context)
> +              have_context = ! STREQ ("_", f->scontext);

You need braces around the above to associate the if else clauses correctly.

thanks,
Pádraig.