[PATCH] audit: shorten PATH cap values when zero

Discussion:

Richard Guy Briggs

2018-11-16 17:10:01 UTC

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 <***@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

Steve Grubb

2018-11-19 04:59:40 UTC

Permalink

Ack.

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

-Steve

Post by Richard Guy Briggs
---
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);
}
}

Paul Moore

2018-11-19 21:22:57 UTC

Permalink

Post by Richard Guy Briggs
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
---
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.

Post by Richard Guy Briggs
+ 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.

Post by Richard Guy Briggs
}
}

--
paul moore
www.paul-moore.com

Richard Guy Briggs

2018-11-19 21:51:53 UTC

Permalink

Post by Paul Moore

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.

Post by Paul Moore

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

Post by Paul Moore

Post by Richard Guy Briggs
}
}

paul moore

- RGB

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

2018-11-19 22:05:01 UTC

Permalink

Post by Richard Guy Briggs

Post by Paul Moore

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.

Post by Paul Moore

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", ...);

Post by Richard Guy Briggs

Post by Paul Moore

Post by Richard Guy Briggs
}
}

paul moore

- RGB
--
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