On Mon, Nov 19, 2018 at 4:52 PM Richard Guy Briggs <rgb(a)redhat.com> wrote:
 On 2018-11-19 16:22, Paul Moore wrote:
 > On Fri, Nov 16, 2018 at 12:10 PM Richard Guy Briggs <rgb(a)redhat.com> wrote:
 > >
 > > Since the vast majority of files (99.993% on a typical system) have no
 > > fcaps, display "0" instead of the full zero-padded 16 hex digits in
the
 > > two PATH record cap_f* fields to save netlink bandwidth and disk space.
 > >
 > > Simply changing the format to %x won't work since the value is two (or
 > > possibly more in the future) 32-bit hexadecimal values concatenated and
 > > bits in higher order values will be misrepresented.
 > >
 > > Passes audit-testsuite and userspace tools already work fine.
 > > Please see the github issue tracker for more details
 > > 
https://github.com/linux-audit/audit-kernel/issues/101
 > >
 > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
 > > ---
 > >  kernel/audit.c | 15 ++++++++++-----
 > >  1 file changed, 10 insertions(+), 5 deletions(-)
 > >
 > > diff --git a/kernel/audit.c b/kernel/audit.c
 > > index 2a8058764aa6..90cbc89fd6d2 100644
 > > --- a/kernel/audit.c
 > > +++ b/kernel/audit.c
 > > @@ -2057,11 +2057,16 @@ void audit_log_key(struct audit_buffer *ab, char *key)
 > >  void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
 > >  {
 > >         int i;
 > > -
 > > -       audit_log_format(ab, " %s=", prefix);
 > > -       CAP_FOR_EACH_U32(i) {
 > > -               audit_log_format(ab, "%08x",
 > > -                                cap->cap[CAP_LAST_U32 - i]);
 > > +       u32 nonzero = 0;
 > > +
 > > +       CAP_FOR_EACH_U32(i)
 > > +               nonzero |= cap->cap[CAP_LAST_U32 - i];
 > > +       if (nonzero) {
 >
 > Let's use the existing cap_isclear() instead.  I'll admit it is pretty
 > trivial, but somebody is surely going to complain about the
 > open-coding at some point so let's fix it now.
 Yes, thank you, I like that much better.
 > > +               audit_log_format(ab, " %s=", prefix);
 > > +               CAP_FOR_EACH_U32(i)
 > > +                       audit_log_format(ab, "%08x",
cap->cap[CAP_LAST_U32 - i]);
 > > +       } else {
 > > +               audit_log_format(ab, " %s=0", prefix);
 >
 > Since you're doing a re-spin, you might as well drop the curly braces
 > here, they aren't needed.
 I agree that is efficient legal C, but that will violate
 Documentation/process/coding-style.rst:191 
  if (cap_isclear(...)) {
    audit_log_format(ab, " %s=0", prefix);
    return;
  }
  audit_log_format(ab, "%s=", prefix);
  CAP_FOR_EACH_U32(i)
    audit_log_format(ab, "%08x", ...);
 > >         }
 > >  }
 >
 > paul moore
 - RGB
 --
 Richard Guy Briggs <rgb(a)redhat.com>
 Sr. S/W Engineer, Kernel Security, Base Operating Systems
 Remote, Ottawa, Red Hat Canada
 IRC: rgb, SunRaycer
 Voice: +1.647.777.2635, Internal: (81) 32635 
-- 
paul moore
www.paul-moore.com