3:42 p.m.
From: Tony Jones <tonyj(a)suse.de>
Minor performance enhancement.
Thread flag TIF_SYSCALL_AUDIT is not cleared for new children when audit
context creation has been disabled (auditctl -e0). This can cause new children
forked from a parent created when audit was enabled to not take the fastest
syscall path thru entry.S
Signed-off-by: Tony Jones <tonyj(a)suse.de>
---
kernel/auditsc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -814,8 +814,10 @@ int audit_alloc(struct task_struct *tsk)
struct audit_context *context;
enum audit_state state;
- if (likely(!audit_enabled))
+ if (likely(!audit_enabled)) {
+ clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
return 0; /* Return if not auditing. */
+ }
state = audit_filter_task(tsk);
if (likely(state == AUDIT_DISABLED))
9:21 a.m.
On Friday 26 October 2007 04:42:28 pm Tony Jones wrote:
Thread flag TIF_SYSCALL_AUDIT is not cleared for new children when
audit
context creation has been disabled (auditctl -e0). This can cause new
children forked from a parent created when audit was enabled to not take
the fastest syscall path thru entry.S
This came up almost 2 years ago:
https://www.redhat.com/archives/linux-audit/2005-September/msg00048.html
The problem is that removing that flag makes the children unauditable in the
future. The only place that flag gets set is during fork. Unless I'm missing
something, to make all children auditable again would mean stopping all
processes and or'ing that flag into all thread info areas. I do not want to
propose that patch to LKML. :)
-Steve
12:20 p.m.
On Sat, Oct 27, 2007 at 10:21:39AM -0400, Steve Grubb wrote:
On Friday 26 October 2007 04:42:28 pm Tony Jones wrote:
> Thread flag TIF_SYSCALL_AUDIT is not cleared for new children when audit
> context creation has been disabled (auditctl -e0). This can cause new
> children forked from a parent created when audit was enabled to not take
> the fastest syscall path thru entry.S
This came up almost 2 years ago:
https://www.redhat.com/archives/linux-audit/2005-September/msg00048.html
I was not aware of this, thanks.
The problem is that removing that flag makes the children unauditable
in the
future. The only place that flag gets set is during fork.
I don't see this. The case that would be undesirable would be for a task
to have an audit context but to not have the thread flag enabled. That isn't
the case. This was the point Chris made in his Ack, although perhaps somewhat
tersely.
Unless I'm missing something, to make all children auditable
again would
mean stopping all processes and or'ing that flag into all thread info areas.
I think you are. Or maybe the code was different two years ago so that the
above made sense.
In the above scenario, audit is disabled, a new child is forked, we bail
early so there is no audit context (and now there is no flag in the thread
area). Currently there is no way this task is ever going to be audited as
there is no audit context. If this task forks a new child, at this point
the value of audit enabled will determine if there should be a context
allocated and it will allocate the TIF flag also. I don't see your stopping
all processes scenario.
I do not want to propose that patch to LKML. :)
;-)
Tony
5:04 p.m.
On Monday 29 October 2007 01:20:58 pm Tony Jones wrote:
> The problem is that removing that flag makes the children
unauditable in
> the future. The only place that flag gets set is during fork.
I don't see this.
If the child does not have the TIF_SYSCALL_AUDIT flag, it never goes into
audit_syscall_entry. It becomes unauditable.
The case that would be undesirable would be for a task to have an
audit
context but to not have the thread flag enabled.
That would just be a small allocation of memory that will be returned when the
process exits. From an auditing PoV, something that is undesirable is the
inability to audit a process that you want to audit.
> Unless I'm missing something, to make all children auditable
again would
> mean stopping all processes and or'ing that flag into all thread info
> areas.
I think you are. Or maybe the code was different two years ago so that the
above made sense.
In the above scenario, audit is disabled, a new child is forked, we bail
early so there is no audit context (and now there is no flag in the thread
area). Currently there is no way this task is ever going to be audited as
there is no audit context.
So when audit is re-enabled, how do you make that task auditable?
If this task forks a new child, at this point the value of audit
enabled
will determine if there should be a context allocated and it will allocate
the TIF flag also.
In the new child, but not the parent.
I don't see your stopping all processes scenario.
That is so you can walk the process table and "or" the bit in unconditionally.
All processes need to be auditable or you've got a security hole.
-Steve
6:15 p.m.
On Mon, Oct 29, 2007 at 06:04:31PM -0400, Steve Grubb wrote:
If the child does not have the TIF_SYSCALL_AUDIT flag, it never goes
into
audit_syscall_entry. It becomes unauditable.
True but a task where current->audit_context == NULL is going to immediately
BUG out in audit_syscall_entry. This is why the invocations of
audit_syscall_entry() are conditional on current->audit_context.
So when audit is re-enabled, how do you make that task auditable?
No idea. How do you do it currently? HINT: current->audit_context == NULL
for these tasks. If !audit_enabled, then audit_alloc() is not going to
allocate an audit_context for the task.
I'm very curious how you think one of these tasks becomes auditable later
on once audit is re-enabled, regardless of the value of TIF_SYSCALL_AUDIT.
Tony
9:33 a.m.
On Monday 29 October 2007 07:15:30 pm Tony Jones wrote:
On Mon, Oct 29, 2007 at 06:04:31PM -0400, Steve Grubb wrote:
> So when audit is re-enabled, how do you make that task auditable?
No idea. How do you do it currently? HINT: current->audit_context == NULL
for these tasks. If !audit_enabled, then audit_alloc() is not going to
allocate an audit_context for the task.
We are looking into this - at one time it did. Someone should follow up with a
path correcting this soon. But I doubt the audit system will work correctly
if the flag gets removed as there is no good way to add it again later.
-Steve
12:23 p.m.
On Thu, Nov 01, 2007 at 10:33:52AM -0400, Steve Grubb wrote:
On Monday 29 October 2007 07:15:30 pm Tony Jones wrote:
> On Mon, Oct 29, 2007 at 06:04:31PM -0400, Steve Grubb wrote:
> > So when audit is re-enabled, how do you make that task auditable?
>
> No idea. How do you do it currently? HINT: current->audit_context == NULL
> for these tasks. If !audit_enabled, then audit_alloc() is not going to
> allocate an audit_context for the task.
We are looking into this - at one time it did. Someone should follow up with
a path correcting this soon. But I doubt the audit system will work correctly
if the flag gets removed as there is no good way to add it again later.
So on the syscall path you're now going to something like (pseudocode):
if (unlikely(current->audit_context)) {audit_syscall_entry()}
else if (audit_enabled) {audit_alloc(); audit_syscall_entry()}
I agree, if this is what you want to do, then clearing the thread flag would
be a bad idea. I'd assumed the current behaviour was by design as allocating
contexts at syscall time doesn't seem a great idea but if you need the
functionality, you need the functionality.
Tony
1:34 p.m.
On Thursday 01 November 2007 01:23:24 pm Tony Jones wrote:
> We are looking into this - at one time it did. Someone should
follow up
> with a path correcting this soon. But I doubt the audit system will work
> correctly if the flag gets removed as there is no good way to add it
> again later.
So on the syscall path you're now going to something like (pseudocode):
if (unlikely(current->audit_context)) {audit_syscall_entry()}
else if (audit_enabled) {audit_alloc(); audit_syscall_entry()}
Something like that. The TIF flag is to pick off processes that we are
interested in auditing, for performance reasons if audit is disabled the
context is not created. But if auditing is re-enabled, processes can
be "repaired" so that the are auditable again by allocating the context on
the fly the first time we see a process. Without the flag, they never get
steered into the audit system where this can happen.
I agree, if this is what you want to do, then clearing the thread
flag
would be a bad idea.
If any other performance improvements jumps out at you, please bring them up.
I'd assumed the current behaviour was by design as allocating
contexts at
syscall time doesn't seem a great idea but if you need the functionality,
you need the functionality.
Yep
-Steve
6260
days inactive
6266
days old
linux-audit@lists.linux-audit.osci.io
7 comments
2 participants
participants (2)
-
Steve Grubb -
Tony Jones