Discussion:
[PATCH V2 1/3] audit: deprecate the AUDIT_FILTER_ENTRY filter
Richard Guy Briggs
2018-02-10 02:40:30 UTC
Permalink
The audit entry filter has been long deprecated with userspace support
finally removed in audit-v2.6.7 and plans to remove kernel support have
existed since kernel-v2.6.31.
Remove it.

Passes audit-testsuite.

See: https://github.com/linux-audit/audit-kernel/issues/6
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/auditfilter.c | 4 ++--
kernel/auditsc.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 4a1758a..1bbf5de 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -258,8 +258,8 @@ static inline struct audit_entry *audit_to_entry_common(struct audit_rule_data *
goto exit_err;
#ifdef CONFIG_AUDITSYSCALL
case AUDIT_FILTER_ENTRY:
- if (rule->action == AUDIT_ALWAYS)
- goto exit_err;
+ pr_err("AUDIT_FILTER_ENTRY is deprecated\n");
+ goto exit_err;
case AUDIT_FILTER_EXIT:
case AUDIT_FILTER_TASK:
#endif
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e80459f..9348302 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1530,7 +1530,8 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
context->dummy = !audit_n_rules;
if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
context->prio = 0;
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY]);
+ if (auditd_test_task(tsk))
+ return;
}
if (state == AUDIT_DISABLED)
return;
--
1.8.3.1
Richard Guy Briggs
2018-02-10 02:40:31 UTC
Permalink
Since removing the audit entry filter, test for early return before
setting up any context state.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/auditsc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9348302..bc534bf 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1519,23 +1519,23 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
if (!audit_enabled)
return;

- context->arch = syscall_get_arch();
- context->major = major;
- context->argv[0] = a1;
- context->argv[1] = a2;
- context->argv[2] = a3;
- context->argv[3] = a4;
-
state = context->state;
+ if (state == AUDIT_DISABLED)
+ return;
+
context->dummy = !audit_n_rules;
if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
context->prio = 0;
if (auditd_test_task(tsk))
return;
}
- if (state == AUDIT_DISABLED)
- return;

+ context->arch = syscall_get_arch();
+ context->major = major;
+ context->argv[0] = a1;
+ context->argv[1] = a2;
+ context->argv[2] = a3;
+ context->argv[3] = a4;
context->serial = 0;
context->ctime = current_kernel_time64();
context->in_syscall = 1;
--
1.8.3.1
Paul Moore
2018-02-14 21:09:42 UTC
Permalink
Post by Richard Guy Briggs
Since removing the audit entry filter, test for early return before
setting up any context state.
---
kernel/auditsc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
Sigh.

First off, thanks for making the changes, I think the end result of
1/3+2/3 is better than the v1 patch.

However, this really didn't need to be two patches, please combine 1/3
and 2/3 and resubmit. I know I've done the patch squashing for you in
the past, but I think it's time to start pushing some of this work
back to you.

Moving forward, if I provide feedback and do not explicitly suggest
creating a new patch, please incorporate the changes into the existing
patches.
Post by Richard Guy Briggs
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9348302..bc534bf 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1519,23 +1519,23 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
if (!audit_enabled)
return;
- context->arch = syscall_get_arch();
- context->major = major;
- context->argv[0] = a1;
- context->argv[1] = a2;
- context->argv[2] = a3;
- context->argv[3] = a4;
-
state = context->state;
+ if (state == AUDIT_DISABLED)
+ return;
+
context->dummy = !audit_n_rules;
if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
context->prio = 0;
if (auditd_test_task(tsk))
return;
}
- if (state == AUDIT_DISABLED)
- return;
+ context->arch = syscall_get_arch();
+ context->major = major;
+ context->argv[0] = a1;
+ context->argv[1] = a2;
+ context->argv[2] = a3;
+ context->argv[3] = a4;
context->serial = 0;
context->ctime = current_kernel_time64();
context->in_syscall = 1;
--
1.8.3.1
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2018-02-10 02:40:32 UTC
Permalink
If audit is disabled, who cares if there is a bug indicating syscall in
process or names already recorded. Bail immediately on audit disabled.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/auditsc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index bc534bf..4e0a4ac 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1511,14 +1511,11 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
struct audit_context *context = tsk->audit_context;
enum audit_state state;

- if (!context)
+ if (!audit_enabled || !context)
return;

BUG_ON(context->in_syscall || context->name_count);

- if (!audit_enabled)
- return;
-
state = context->state;
if (state == AUDIT_DISABLED)
return;
--
1.8.3.1
Loading...