All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] audit: shorten PATH cap values when zero
@ 2018-11-16 17:10 Richard Guy Briggs
  2018-11-19  4:59 ` Steve Grubb
  2018-11-19 21:22 ` Paul Moore
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Guy Briggs @ 2018-11-16 17:10 UTC (permalink / raw)
  To: Linux-Audit Mailing List

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@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) {
+		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);
 	}
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: shorten PATH cap values when zero
  2018-11-16 17:10 [PATCH] audit: shorten PATH cap values when zero Richard Guy Briggs
@ 2018-11-19  4:59 ` Steve Grubb
  2018-11-19 21:22 ` Paul Moore
  1 sibling, 0 replies; 5+ messages in thread
From: Steve Grubb @ 2018-11-19  4:59 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Linux-Audit Mailing List

On Friday, November 16, 2018 12:10:01 PM EST Richard Guy Briggs 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@redhat.com>

Ack. 

Thanks for doing this. It helps to alleviate my concern about wasted disk 
space for normal files.

-Steve

> ---
>  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) {
> +		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);
>  	}
>  }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: shorten PATH cap values when zero
  2018-11-16 17:10 [PATCH] audit: shorten PATH cap values when zero Richard Guy Briggs
  2018-11-19  4:59 ` Steve Grubb
@ 2018-11-19 21:22 ` Paul Moore
  2018-11-19 21:51   ` Richard Guy Briggs
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Moore @ 2018-11-19 21:22 UTC (permalink / raw)
  To: rgb; +Cc: linux-audit

On Fri, Nov 16, 2018 at 12:10 PM Richard Guy Briggs <rgb@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@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.

> +               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.

>         }
>  }

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: shorten PATH cap values when zero
  2018-11-19 21:22 ` Paul Moore
@ 2018-11-19 21:51   ` Richard Guy Briggs
  2018-11-19 22:05     ` Paul Moore
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Guy Briggs @ 2018-11-19 21:51 UTC (permalink / raw)
  To: Paul Moore; +Cc: linux-audit

On 2018-11-19 16:22, Paul Moore wrote:
> On Fri, Nov 16, 2018 at 12:10 PM Richard Guy Briggs <rgb@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@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

> >         }
> >  }
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] audit: shorten PATH cap values when zero
  2018-11-19 21:51   ` Richard Guy Briggs
@ 2018-11-19 22:05     ` Paul Moore
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2018-11-19 22:05 UTC (permalink / raw)
  To: rgb; +Cc: linux-audit

On Mon, Nov 19, 2018 at 4:52 PM Richard Guy Briggs <rgb@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@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@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@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-11-19 22:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16 17:10 [PATCH] audit: shorten PATH cap values when zero Richard Guy Briggs
2018-11-19  4:59 ` Steve Grubb
2018-11-19 21:22 ` Paul Moore
2018-11-19 21:51   ` Richard Guy Briggs
2018-11-19 22:05     ` Paul Moore

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.