Discussion:
[PATCH ghak59 V1 0/2] tree and watch rule log cleanups
Richard Guy Briggs
2018-06-14 20:20:04 UTC
Permalink
Make some tree and watch rule logging cleanups before applying
normalizations and record connections for ghak 59.

See: https://github.com/linux-audit/audit-kernel/issues/50

Richard Guy Briggs (2):
audit: tree: check audit_enabled
audit: watch: simplify audit_enabled check

kernel/audit_tree.c | 2 ++
kernel/audit_watch.c | 29 +++++++++++++++--------------
2 files changed, 17 insertions(+), 14 deletions(-)
--
1.8.3.1
Richard Guy Briggs
2018-06-14 20:20:06 UTC
Permalink
Check the audit_enabled flag and bail immediately. This does not change
the functionality, but brings the code format in line with similar
checks in audit_tree_log_remove_rule(), audit_mark_log_rule_change(),
and elsewhere in the audit code.

See: https://github.com/linux-audit/audit-kernel/issues/50
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/audit_watch.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f1ba889..9b4836b 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -238,20 +238,21 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)

static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
{
- if (audit_enabled) {
- struct audit_buffer *ab;
- ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
- if (unlikely(!ab))
- return;
- audit_log_format(ab, "auid=%u ses=%u op=%s",
- from_kuid(&init_user_ns, audit_get_loginuid(current)),
- audit_get_sessionid(current), op);
- audit_log_format(ab, " path=");
- audit_log_untrustedstring(ab, w->path);
- audit_log_key(ab, r->filterkey);
- audit_log_format(ab, " list=%d res=1", r->listnr);
- audit_log_end(ab);
- }
+ struct audit_buffer *ab;
+
+ if (!audit_enabled)
+ return;
+ ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+ if (!ab)
+ return;
+ audit_log_format(ab, "auid=%u ses=%u op=%s",
+ from_kuid(&init_user_ns, audit_get_loginuid(current)),
+ audit_get_sessionid(current), op);
+ audit_log_format(ab, " path=");
+ audit_log_untrustedstring(ab, w->path);
+ audit_log_key(ab, r->filterkey);
+ audit_log_format(ab, " list=%d res=1", r->listnr);
+ audit_log_end(ab);
}

/* Update inode info in audit rules based on filesystem event. */
--
1.8.3.1
Paul Moore
2018-06-28 15:47:47 UTC
Permalink
Post by Richard Guy Briggs
Check the audit_enabled flag and bail immediately. This does not change
the functionality, but brings the code format in line with similar
checks in audit_tree_log_remove_rule(), audit_mark_log_rule_change(),
and elsewhere in the audit code.
See: https://github.com/linux-audit/audit-kernel/issues/50
---
kernel/audit_watch.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
Merged, thanks.

As a FYI for future patches, please don't use "audit: X: <one-liner>"
as a subject line unless you are crossing subsystem boundaries. As an
example, the following is okay:

audit: selinux: make things more awesomer

... while this isn't something I like seeing:

audit: watch: simplify audit_enabled check

... because the "watch" in this case refers to the audit watch code
which is part of the audit subsystem already.
Post by Richard Guy Briggs
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f1ba889..9b4836b 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -238,20 +238,21 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
{
- if (audit_enabled) {
- struct audit_buffer *ab;
- ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
- if (unlikely(!ab))
- return;
- audit_log_format(ab, "auid=%u ses=%u op=%s",
- from_kuid(&init_user_ns, audit_get_loginuid(current)),
- audit_get_sessionid(current), op);
- audit_log_format(ab, " path=");
- audit_log_untrustedstring(ab, w->path);
- audit_log_key(ab, r->filterkey);
- audit_log_format(ab, " list=%d res=1", r->listnr);
- audit_log_end(ab);
- }
+ struct audit_buffer *ab;
+
+ if (!audit_enabled)
+ return;
+ ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+ if (!ab)
+ return;
+ audit_log_format(ab, "auid=%u ses=%u op=%s",
+ from_kuid(&init_user_ns, audit_get_loginuid(current)),
+ audit_get_sessionid(current), op);
+ audit_log_format(ab, " path=");
+ audit_log_untrustedstring(ab, w->path);
+ audit_log_key(ab, r->filterkey);
+ audit_log_format(ab, " list=%d res=1", r->listnr);
+ audit_log_end(ab);
}
/* Update inode info in audit rules based on filesystem event. */
--
1.8.3.1
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2018-07-13 15:39:19 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Check the audit_enabled flag and bail immediately. This does not change
the functionality, but brings the code format in line with similar
checks in audit_tree_log_remove_rule(), audit_mark_log_rule_change(),
and elsewhere in the audit code.
See: https://github.com/linux-audit/audit-kernel/issues/50
---
kernel/audit_watch.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
Merged, thanks.
As a FYI for future patches, please don't use "audit: X: <one-liner>"
as a subject line unless you are crossing subsystem boundaries. As an
audit: selinux: make things more awesomer
audit: watch: simplify audit_enabled check
... because the "watch" in this case refers to the audit watch code
which is part of the audit subsystem already.
Ok, so that watch keyword should have been used such as:
"audit: simplify watch audit_enabled check"

I had seen and used it as a sub-sub-system tag rather than an additional
sub-system tag.

Thanks.
Post by Paul Moore
Post by Richard Guy Briggs
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f1ba889..9b4836b 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -238,20 +238,21 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
{
- if (audit_enabled) {
- struct audit_buffer *ab;
- ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
- if (unlikely(!ab))
- return;
- audit_log_format(ab, "auid=%u ses=%u op=%s",
- from_kuid(&init_user_ns, audit_get_loginuid(current)),
- audit_get_sessionid(current), op);
- audit_log_format(ab, " path=");
- audit_log_untrustedstring(ab, w->path);
- audit_log_key(ab, r->filterkey);
- audit_log_format(ab, " list=%d res=1", r->listnr);
- audit_log_end(ab);
- }
+ struct audit_buffer *ab;
+
+ if (!audit_enabled)
+ return;
+ ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+ if (!ab)
+ return;
+ audit_log_format(ab, "auid=%u ses=%u op=%s",
+ from_kuid(&init_user_ns, audit_get_loginuid(current)),
+ audit_get_sessionid(current), op);
+ audit_log_format(ab, " path=");
+ audit_log_untrustedstring(ab, w->path);
+ audit_log_key(ab, r->filterkey);
+ audit_log_format(ab, " list=%d res=1", r->listnr);
+ audit_log_end(ab);
}
/* Update inode info in audit rules based on filesystem event. */
--
1.8.3.1
--
paul moore
www.paul-moore.com
- 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

Richard Guy Briggs
2018-06-14 20:20:05 UTC
Permalink
Respect the audit_enabled flag when printing tree rule config change
records.

See: https://github.com/linux-audit/audit-kernel/issues/50
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/audit_tree.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 67e6956..5e9d1e5 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -497,6 +497,8 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
{
struct audit_buffer *ab;

+ if (!audit_enabled)
+ return;
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (unlikely(!ab))
return;
--
1.8.3.1
Paul Moore
2018-06-28 15:43:28 UTC
Permalink
Post by Richard Guy Briggs
Respect the audit_enabled flag when printing tree rule config change
records.
See: https://github.com/linux-audit/audit-kernel/issues/50
---
kernel/audit_tree.c | 2 ++
1 file changed, 2 insertions(+)
Merged, thanks.
Post by Richard Guy Briggs
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 67e6956..5e9d1e5 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -497,6 +497,8 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
{
struct audit_buffer *ab;
+ if (!audit_enabled)
+ return;
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (unlikely(!ab))
return;
--
1.8.3.1
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2018-06-14 21:01:33 UTC
Permalink
Post by Richard Guy Briggs
Make some tree and watch rule logging cleanups before applying
normalizations and record connections for ghak 59.
See: https://github.com/linux-audit/audit-kernel/issues/50
Sorry, this patchset is mislabelled in the subject line and should be
ghak50.
Post by Richard Guy Briggs
audit: tree: check audit_enabled
audit: watch: simplify audit_enabled check
kernel/audit_tree.c | 2 ++
kernel/audit_watch.c | 29 +++++++++++++++--------------
2 files changed, 17 insertions(+), 14 deletions(-)
--
1.8.3.1
- 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
Continue reading on narkive:
Loading...