public inbox for linux-audit@redhat.com
 help / color / mirror / Atom feed
* [patch 058/209] audit: rework execve audit
@ 2007-07-19  8:48 akpm
  2007-07-27 20:13 ` Steve Grubb
  0 siblings, 1 reply; 14+ messages in thread
From: akpm @ 2007-07-19  8:48 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, linux-audit, a.p.zijlstra, aaw

From: Peter Zijlstra <a.p.zijlstra@chello.nl>

The purpose of audit_bprm() is to log the argv array to a userspace daemon at
the end of the execve system call.  Since user-space hasn't had time to run,
this array is still in pristine state on the process' stack; so no need to
copy it, we can just grab it from there.

In order to minimize the damage to audit_log_*() copy each string into a
temporary kernel buffer first.

Currently the audit code requires that the full argument vector fits in a
single packet.  So currently it does clip the argv size to a (sysctl) limit,
but only when execve auditing is enabled.

If the audit protocol gets extended to allow for multiple packets this check
can be removed.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Ollie Wild <aaw@google.com>
Cc: <linux-audit@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 Documentation/filesystems/proc.txt |    7 ++
 fs/exec.c                          |    3 
 include/linux/binfmts.h            |    1 
 kernel/auditsc.c                   |   84 ++++++++++++++++++++-------
 kernel/sysctl.c                    |   11 +++
 5 files changed, 85 insertions(+), 21 deletions(-)

diff -puN Documentation/filesystems/proc.txt~audit-rework-execve-audit Documentation/filesystems/proc.txt
--- a/Documentation/filesystems/proc.txt~audit-rework-execve-audit
+++ a/Documentation/filesystems/proc.txt
@@ -1065,6 +1065,13 @@ check the amount of free space (value is
 resume it  if we have a value of 3 or more percent; consider information about
 the amount of free space valid for 30 seconds
 
+audit_argv_kb
+-------------
+
+The file contains a single value denoting the limit on the argv array size
+for execve (in KiB). This limit is only applied when system call auditing for
+execve is enabled, otherwise the value is ignored.
+
 ctrl-alt-del
 ------------
 
diff -puN fs/exec.c~audit-rework-execve-audit fs/exec.c
--- a/fs/exec.c~audit-rework-execve-audit
+++ a/fs/exec.c
@@ -1154,6 +1154,7 @@ int do_execve(char * filename,
 {
 	struct linux_binprm *bprm;
 	struct file *file;
+	unsigned long env_p;
 	int retval;
 	int i;
 
@@ -1208,9 +1209,11 @@ int do_execve(char * filename,
 	if (retval < 0)
 		goto out;
 
+	env_p = bprm->p;
 	retval = copy_strings(bprm->argc, argv, bprm);
 	if (retval < 0)
 		goto out;
+	bprm->argv_len = env_p - bprm->p;
 
 	retval = search_binary_handler(bprm,regs);
 	if (retval >= 0) {
diff -puN include/linux/binfmts.h~audit-rework-execve-audit include/linux/binfmts.h
--- a/include/linux/binfmts.h~audit-rework-execve-audit
+++ a/include/linux/binfmts.h
@@ -40,6 +40,7 @@ struct linux_binprm{
 	unsigned interp_flags;
 	unsigned interp_data;
 	unsigned long loader, exec;
+	unsigned long argv_len;
 };
 
 #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0
diff -puN kernel/auditsc.c~audit-rework-execve-audit kernel/auditsc.c
--- a/kernel/auditsc.c~audit-rework-execve-audit
+++ a/kernel/auditsc.c
@@ -153,7 +153,7 @@ struct audit_aux_data_execve {
 	struct audit_aux_data	d;
 	int argc;
 	int envc;
-	char mem[0];
+	struct mm_struct *mm;
 };
 
 struct audit_aux_data_socketcall {
@@ -831,6 +831,55 @@ static int audit_log_pid_context(struct 
 	return rc;
 }
 
+static void audit_log_execve_info(struct audit_buffer *ab,
+		struct audit_aux_data_execve *axi)
+{
+	int i;
+	long len, ret;
+	const char __user *p = (const char __user *)axi->mm->arg_start;
+	char *buf;
+
+	if (axi->mm != current->mm)
+		return; /* execve failed, no additional info */
+
+	for (i = 0; i < axi->argc; i++, p += len) {
+		len = strnlen_user(p, MAX_ARG_PAGES*PAGE_SIZE);
+		/*
+		 * We just created this mm, if we can't find the strings
+		 * we just copied into it something is _very_ wrong. Similar
+		 * for strings that are too long, we should not have created
+		 * any.
+		 */
+		if (!len || len > MAX_ARG_STRLEN) {
+			WARN_ON(1);
+			send_sig(SIGKILL, current, 0);
+		}
+
+		buf = kmalloc(len, GFP_KERNEL);
+		if (!buf) {
+			audit_panic("out of memory for argv string\n");
+			break;
+		}
+
+		ret = copy_from_user(buf, p, len);
+		/*
+		 * There is no reason for this copy to be short. We just
+		 * copied them here, and the mm hasn't been exposed to user-
+		 * space yet.
+		 */
+		if (!ret) {
+			WARN_ON(1);
+			send_sig(SIGKILL, current, 0);
+		}
+
+		audit_log_format(ab, "a%d=", i);
+		audit_log_untrustedstring(ab, buf);
+		audit_log_format(ab, "\n");
+
+		kfree(buf);
+	}
+}
+
 static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)
 {
 	int i, call_panic = 0;
@@ -971,13 +1020,7 @@ static void audit_log_exit(struct audit_
 
 		case AUDIT_EXECVE: {
 			struct audit_aux_data_execve *axi = (void *)aux;
-			int i;
-			const char *p;
-			for (i = 0, p = axi->mem; i < axi->argc; i++) {
-				audit_log_format(ab, "a%d=", i);
-				p = audit_log_untrustedstring(ab, p);
-				audit_log_format(ab, "\n");
-			}
+			audit_log_execve_info(ab, axi);
 			break; }
 
 		case AUDIT_SOCKETCALL: {
@@ -1821,32 +1864,31 @@ int __audit_ipc_set_perm(unsigned long q
 	return 0;
 }
 
+int audit_argv_kb = 32;
+
 int audit_bprm(struct linux_binprm *bprm)
 {
 	struct audit_aux_data_execve *ax;
 	struct audit_context *context = current->audit_context;
-	unsigned long p, next;
-	void *to;
 
 	if (likely(!audit_enabled || !context || context->dummy))
 		return 0;
 
-	ax = kmalloc(sizeof(*ax) + PAGE_SIZE * MAX_ARG_PAGES - bprm->p,
-				GFP_KERNEL);
+	/*
+	 * Even though the stack code doesn't limit the arg+env size any more,
+	 * the audit code requires that _all_ arguments be logged in a single
+	 * netlink skb. Hence cap it :-(
+	 */
+	if (bprm->argv_len > (audit_argv_kb << 10))
+		return -E2BIG;
+
+	ax = kmalloc(sizeof(*ax), GFP_KERNEL);
 	if (!ax)
 		return -ENOMEM;
 
 	ax->argc = bprm->argc;
 	ax->envc = bprm->envc;
-	for (p = bprm->p, to = ax->mem; p < MAX_ARG_PAGES*PAGE_SIZE; p = next) {
-		struct page *page = bprm->page[p / PAGE_SIZE];
-		void *kaddr = kmap(page);
-		next = (p + PAGE_SIZE) & ~(PAGE_SIZE - 1);
-		memcpy(to, kaddr + (p & (PAGE_SIZE - 1)), next - p);
-		to += next - p;
-		kunmap(page);
-	}
-
+	ax->mm = bprm->mm;
 	ax->d.type = AUDIT_EXECVE;
 	ax->d.next = context->aux;
 	context->aux = (void *)ax;
diff -puN kernel/sysctl.c~audit-rework-execve-audit kernel/sysctl.c
--- a/kernel/sysctl.c~audit-rework-execve-audit
+++ a/kernel/sysctl.c
@@ -78,6 +78,7 @@ extern int percpu_pagelist_fraction;
 extern int compat_log;
 extern int maps_protect;
 extern int sysctl_stat_interval;
+extern int audit_argv_kb;
 
 /* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
 static int maxolduid = 65535;
@@ -306,6 +307,16 @@ static ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+#ifdef CONFIG_AUDITSYSCALL
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "audit_argv_kb",
+		.data		= &audit_argv_kb,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
+#endif
 	{
 		.ctl_name	= KERN_CORE_PATTERN,
 		.procname	= "core_pattern",
_

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-19  8:48 [patch 058/209] audit: rework execve audit akpm
@ 2007-07-27 20:13 ` Steve Grubb
  2007-07-27 20:44   ` Peter Zijlstra
                     ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Steve Grubb @ 2007-07-27 20:13 UTC (permalink / raw)
  To: linux-audit; +Cc: aaw, a.p.zijlstra

Hi,

I was testing our rawhide kernel and I'm scrolling these errors:

WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)

Call Trace:
 [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
 [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
 [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
 [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
 [<ffffffff8100beb1>] int_very_careful+0x3a/0x43


> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> diff -puN kernel/auditsc.c~audit-rework-execve-audit kernel/auditsc.c
> --- a/kernel/auditsc.c~audit-rework-execve-audit
> +++ a/kernel/auditsc.c
> @@ -831,6 +831,55 @@ static int audit_log_pid_context(struct
>  	return rc;
>  }
>
> +static void audit_log_execve_info(struct audit_buffer *ab,
> +		struct audit_aux_data_execve *axi)
> +{
> +	int i;
> +	long len, ret;
> +	const char __user *p = (const char __user *)axi->mm->arg_start;
> +	char *buf;
> +
> +	if (axi->mm != current->mm)
> +		return; /* execve failed, no additional info */
> +
> +	for (i = 0; i < axi->argc; i++, p += len) {
> +		len = strnlen_user(p, MAX_ARG_PAGES*PAGE_SIZE);
> +		/*
> +		 * We just created this mm, if we can't find the strings
> +		 * we just copied into it something is _very_ wrong. Similar
> +		 * for strings that are too long, we should not have created
> +		 * any.
> +		 */
> +		if (!len || len > MAX_ARG_STRLEN) {
> +			WARN_ON(1);
> +			send_sig(SIGKILL, current, 0);
> +		}

Which is right here ^^^

Any ideas?

-Steve

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 20:13 ` Steve Grubb
@ 2007-07-27 20:44   ` Peter Zijlstra
  2007-07-27 20:57     ` Steve Grubb
  2007-07-27 20:50   ` Alexander Viro
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2007-07-27 20:44 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw

On Fri, 2007-07-27 at 16:13 -0400, Steve Grubb wrote:

> I was testing our rawhide kernel and I'm scrolling these errors:

How can I reproduce this? (I once figured out how to enable execve
auditing but have since forgotten)

And are you doing more than enabling it? That is, does it auto-magically
happen, or are you running some tests.

> WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)
> 
> Call Trace:
>  [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
>  [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
>  [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
>  [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
>  [<ffffffff8100beb1>] int_very_careful+0x3a/0x43
> 
> 
> > From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > diff -puN kernel/auditsc.c~audit-rework-execve-audit kernel/auditsc.c
> > --- a/kernel/auditsc.c~audit-rework-execve-audit
> > +++ a/kernel/auditsc.c
> > @@ -831,6 +831,55 @@ static int audit_log_pid_context(struct
> >  	return rc;
> >  }
> >
> > +static void audit_log_execve_info(struct audit_buffer *ab,
> > +		struct audit_aux_data_execve *axi)
> > +{
> > +	int i;
> > +	long len, ret;
> > +	const char __user *p = (const char __user *)axi->mm->arg_start;
> > +	char *buf;
> > +
> > +	if (axi->mm != current->mm)
> > +		return; /* execve failed, no additional info */
> > +
> > +	for (i = 0; i < axi->argc; i++, p += len) {
> > +		len = strnlen_user(p, MAX_ARG_PAGES*PAGE_SIZE);
> > +		/*
> > +		 * We just created this mm, if we can't find the strings
> > +		 * we just copied into it something is _very_ wrong. Similar
> > +		 * for strings that are too long, we should not have created
> > +		 * any.
> > +		 */
> > +		if (!len || len > MAX_ARG_STRLEN) {
> > +			WARN_ON(1);
> > +			send_sig(SIGKILL, current, 0);
> > +		}
> 
> Which is right here ^^^
> 
> Any ideas?

Not from the top of my head, like the comment suggests, its not supposed
to happen :-(. It would be interesting to know if i == 0, if so that
would suggest arg_start is fuzzed, if not something else has gone south.

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 20:13 ` Steve Grubb
  2007-07-27 20:44   ` Peter Zijlstra
@ 2007-07-27 20:50   ` Alexander Viro
  2007-07-27 21:03   ` Alexander Viro
  2007-07-27 22:55   ` [PATCH] audit: fix two bugs in the new execve audit code Peter Zijlstra
  3 siblings, 0 replies; 14+ messages in thread
From: Alexander Viro @ 2007-07-27 20:50 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw, a.p.zijlstra

On Fri, Jul 27, 2007 at 04:13:10PM -0400, Steve Grubb wrote:
> > +		len = strnlen_user(p, MAX_ARG_PAGES*PAGE_SIZE);
> > +		/*
> > +		 * We just created this mm, if we can't find the strings
> > +		 * we just copied into it something is _very_ wrong. Similar
> > +		 * for strings that are too long, we should not have created
> > +		 * any.
> > +		 */
> > +		if (!len || len > MAX_ARG_STRLEN) {
> > +			WARN_ON(1);
> > +			send_sig(SIGKILL, current, 0);
> > +		}
> 
> Which is right here ^^^
> 
> Any ideas?

Empty string in the middle of argv?  Quite legal...
; cat foo.c
#include <stdio.h>
main(int argc, char **argv)
{
	while (argc--) {
		printf("<%d:%s>", strlen(*argv), *argv);
		argv++;
	}
	printf("\n");
	return 0;
}
; gcc foo.c
; ./a.out a b
<7:./a.out><1:a><1:b>
; ./a.out a "" b
<7:./a.out><1:a><0:><1:b>
;

IOW, it's trivial to arrange - len can be 0 just fine...

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 20:44   ` Peter Zijlstra
@ 2007-07-27 20:57     ` Steve Grubb
  2007-07-27 21:55       ` Peter Zijlstra
  0 siblings, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2007-07-27 20:57 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-audit, aaw

On Friday 27 July 2007 16:44:05 Peter Zijlstra wrote:
> On Fri, 2007-07-27 at 16:13 -0400, Steve Grubb wrote:
> > I was testing our rawhide kernel and I'm scrolling these errors:
>
> How can I reproduce this? (I once figured out how to enable execve
> auditing but have since forgotten)

I don't know of anything special its a fully updated rawhide machine. I am not 
running any tests, this is at the prompt in runlevel 3. I have audit=1 as a 
boot parameter in grub.conf and very simple audit rules for that machine:

-D
-b 256
-a exit,always -S sethostname
-w /etc/selinux/config

which is not exotic.


> And are you doing more than enabling it? 

Not really.

> That is, does it auto-magically happen, 

correct...while sitting at the prompt.

> > WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)
> >
> > Call Trace:
> >  [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
> >  [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
> >  [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
> >  [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
> >  [<ffffffff8100beb1>] int_very_careful+0x3a/0x43
> >
> > > From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > diff -puN kernel/auditsc.c~audit-rework-execve-audit kernel/auditsc.c
> > > --- a/kernel/auditsc.c~audit-rework-execve-audit
> > > +++ a/kernel/auditsc.c
> > > @@ -831,6 +831,55 @@ static int audit_log_pid_context(struct
> > >  	return rc;
> > >  }
> > >
> > > +static void audit_log_execve_info(struct audit_buffer *ab,
> > > +		struct audit_aux_data_execve *axi)
> > > +{
> > > +	int i;
> > > +	long len, ret;
> > > +	const char __user *p = (const char __user *)axi->mm->arg_start;
> > > +	char *buf;
> > > +
> > > +	if (axi->mm != current->mm)
> > > +		return; /* execve failed, no additional info */
> > > +
> > > +	for (i = 0; i < axi->argc; i++, p += len) {
> > > +		len = strnlen_user(p, MAX_ARG_PAGES*PAGE_SIZE);
> > > +		/*
> > > +		 * We just created this mm, if we can't find the strings
> > > +		 * we just copied into it something is _very_ wrong. Similar
> > > +		 * for strings that are too long, we should not have created
> > > +		 * any.
> > > +		 */
> > > +		if (!len || len > MAX_ARG_STRLEN) {
> > > +			WARN_ON(1);
> > > +			send_sig(SIGKILL, current, 0);
> > > +		}
> >
> > Which is right here ^^^
> >
> > Any ideas?
>
> Not from the top of my head, like the comment suggests, its not supposed
> to happen :-(. It would be interesting to know if i == 0, if so that
> would suggest arg_start is fuzzed, if not something else has gone south.

Is that all you want is i's value? maybe len too? The trace was awfully short. 
Is there a way to make it tell more about what was in the call chain? IOW, 
tracing back to sys_execve entry.

-Steve

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 20:13 ` Steve Grubb
  2007-07-27 20:44   ` Peter Zijlstra
  2007-07-27 20:50   ` Alexander Viro
@ 2007-07-27 21:03   ` Alexander Viro
  2007-07-27 21:11     ` Steve Grubb
  2007-07-27 22:55   ` [PATCH] audit: fix two bugs in the new execve audit code Peter Zijlstra
  3 siblings, 1 reply; 14+ messages in thread
From: Alexander Viro @ 2007-07-27 21:03 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw, a.p.zijlstra

On Fri, Jul 27, 2007 at 04:13:10PM -0400, Steve Grubb wrote:
> Hi,
> 
> I was testing our rawhide kernel and I'm scrolling these errors:
> 
> WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)
> 
> Call Trace:
>  [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
>  [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
>  [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
>  [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
>  [<ffffffff8100beb1>] int_very_careful+0x3a/0x43

Umm... which architecture?

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 21:03   ` Alexander Viro
@ 2007-07-27 21:11     ` Steve Grubb
  2007-07-27 21:21       ` Alexander Viro
  0 siblings, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2007-07-27 21:11 UTC (permalink / raw)
  To: Alexander Viro; +Cc: linux-audit, aaw, a.p.zijlstra

On Friday 27 July 2007 17:03:59 Alexander Viro wrote:
> On Fri, Jul 27, 2007 at 04:13:10PM -0400, Steve Grubb wrote:
> > WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)
> >
> > Call Trace:
> >  [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
> >  [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
> >  [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
> >  [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
> >  [<ffffffff8100beb1>] int_very_careful+0x3a/0x43
>
> Umm... which architecture?

Its the x86_64 kernel on a Turion64 processor.

-Steve

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 21:11     ` Steve Grubb
@ 2007-07-27 21:21       ` Alexander Viro
  0 siblings, 0 replies; 14+ messages in thread
From: Alexander Viro @ 2007-07-27 21:21 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw, a.p.zijlstra

On Fri, Jul 27, 2007 at 05:11:22PM -0400, Steve Grubb wrote:
> On Friday 27 July 2007 17:03:59 Alexander Viro wrote:
> > On Fri, Jul 27, 2007 at 04:13:10PM -0400, Steve Grubb wrote:
> > > WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)
> > >
> > > Call Trace:
> > >  [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
> > >  [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
> > >  [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
> > >  [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
> > >  [<ffffffff8100beb1>] int_very_careful+0x3a/0x43
> >
> > Umm... which architecture?
> 
> Its the x86_64 kernel on a Turion64 processor.

I think I understand what's going on.  Test for execve failure in
there is bloody bogus.

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 20:57     ` Steve Grubb
@ 2007-07-27 21:55       ` Peter Zijlstra
  2007-07-27 22:06         ` Peter Zijlstra
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2007-07-27 21:55 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw

On Fri, 2007-07-27 at 16:57 -0400, Steve Grubb wrote:

> I don't know of anything special its a fully updated rawhide machine. I am not 
> running any tests, this is at the prompt in runlevel 3. I have audit=1 as a 
> boot parameter in grub.conf and very simple audit rules for that machine:
> 
> -D
> -b 256
> -a exit,always -S sethostname
> -w /etc/selinux/config
> 
> which is not exotic.

I'm feeling dumb,.. on fedora 7 userland I do:

[root@opteron ~]# auditctl -D
No rules
[root@opteron ~]# auditctl -b 256
AUDIT_STATUS: enabled=0 flag=1 pid=0 rate_limit=0 backlog_limit=256 lost=0 backlog=0
[root@opteron ~]# auditctl -a exit,always -S sethostname
Error sending add rule request (Invalid argument)

man auditctl seems to suggest that is a valid command.

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 21:55       ` Peter Zijlstra
@ 2007-07-27 22:06         ` Peter Zijlstra
  2007-07-27 22:24           ` Peter Zijlstra
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2007-07-27 22:06 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw

On Fri, 2007-07-27 at 23:55 +0200, Peter Zijlstra wrote:
> On Fri, 2007-07-27 at 16:57 -0400, Steve Grubb wrote:
> 
> > I don't know of anything special its a fully updated rawhide machine. I am not 
> > running any tests, this is at the prompt in runlevel 3. I have audit=1 as a 
> > boot parameter in grub.conf and very simple audit rules for that machine:
> > 
> > -D
> > -b 256
> > -a exit,always -S sethostname
> > -w /etc/selinux/config
> > 
> > which is not exotic.
> 
> I'm feeling dumb,.. on fedora 7 userland I do:
> 
> [root@opteron ~]# auditctl -D
> No rules
> [root@opteron ~]# auditctl -b 256
> AUDIT_STATUS: enabled=0 flag=1 pid=0 rate_limit=0 backlog_limit=256 lost=0 backlog=0
> [root@opteron ~]# auditctl -a exit,always -S sethostname
> Error sending add rule request (Invalid argument)
> 
> man auditctl seems to suggest that is a valid command.

Ok, I am dumb, CONFIG_AUDITSYSCALL=n

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

* Re: [patch 058/209] audit: rework execve audit
  2007-07-27 22:06         ` Peter Zijlstra
@ 2007-07-27 22:24           ` Peter Zijlstra
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2007-07-27 22:24 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit, aaw

On Sat, 2007-07-28 at 00:06 +0200, Peter Zijlstra wrote:
> On Fri, 2007-07-27 at 23:55 +0200, Peter Zijlstra wrote:
> > On Fri, 2007-07-27 at 16:57 -0400, Steve Grubb wrote:
> > 
> > > I don't know of anything special its a fully updated rawhide machine. I am not 
> > > running any tests, this is at the prompt in runlevel 3. I have audit=1 as a 
> > > boot parameter in grub.conf and very simple audit rules for that machine:
> > > 
> > > -D
> > > -b 256
> > > -a exit,always -S sethostname
> > > -w /etc/selinux/config
> > > 
> > > which is not exotic.


[root@opteron ~]# auditctl -D
No rules
[root@opteron ~]# auditctl -b 256
AUDIT_STATUS: enabled=0 flag=1 pid=0 rate_limit=0 backlog_limit=256 lost=0 backlog=0
[root@opteron ~]# auditctl -a exit,always -S sethostname
[root@opteron ~]# auditctl -w /etc/selinux/config
[root@opteron ~]# man auditd
[root@opteron ~]#  auditd -f
Config file /etc/audit/auditd.conf opened for parsing
log_file_parser called with: /var/log/audit/audit.log
log_format_parser called with: RAW
priority_boost_parser called with: 3
flush_parser called with: INCREMENTAL
freq_parser called with: 20
num_logs_parser called with: 4
dispatch_parser called with: /sbin/audispd
qos_parser called with: lossy
max_log_size_parser called with: 5
max_log_size_action_parser called with: ROTATE
space_left_parser called with: 75
space_action_parser called with: SYSLOG
action_mail_acct_parser called with: root
admin_space_left_parser called with: 50
admin_space_left_action_parser called with: SUSPEND
disk_full_action_parser called with: SUSPEND
disk_error_action_parser called with: SUSPEND
Started dispatcher: /sbin/audispd pid: 3375
type=DAEMON_START msg=audit(1185574384.343:9448) auditd start, ver=1.5.3, format=raw, auid=4294967295 pid=3373 res=success, auditd pid=3373
config_manager init complete
Init complete, auditd 1.5.3 listening for events
type=CONFIG_CHANGE msg=audit(1185574384.450:6): audit_enabled=1 old=0 by auid=4294967295 res=1
type=SYSCALL msg=audit(1185574406.346:7): arch=c000003e syscall=2 success=yes exit=3 a0=2ba34c4f61f6 a1=0 a2=1b6 a3=0 items=1 ppid=2903 pid=3376 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) comm="sshd" exe="/usr/sbin/sshd" key=(null)
type=CWD msg=audit(1185574406.346:7):  cwd="/"
type=PATH msg=audit(1185574406.346:7): item=0 name="/etc/selinux/config" inode=19989869 dev=08:03 mode=0100644 ouid=0 ogid=0 rdev=00:00
type=USER_ACCT msg=audit(1185574406.528:8): user pid=3376 uid=0 auid=4294967295 msg='PAM: accounting acct=root : exe="/usr/sbin/sshd" (hostname=192.168.0.32, addr=192.168.0.32, terminal=ssh res=success)'
...

-----------

when I pressed ctrl-c to try -a exit,always -S execve I found this on my serial console:

-----------
Kernel 2.6.23-rc1 on an x86_64

opteron.programming.kicks-ass.net login: 
[   75.452053] audit(1185574293.834:2): audit_backlog_limit=256 old=64 by auid=4294967295 res=1
[  120.237812] audit(1185574338.691:3): auid=4294967295 op=add rule key=(null) list=4 res=1
[  149.512552] audit(1185574368.012:4): auid=4294967295 op=add rule key=(null) list=4 res=1
[  165.816721] audit(1185574384.343:5): audit_pid=3373 old=0 by auid=4294967295
[  465.113754] Unable to handle kernel NULL pointer dereference at 0000000000000484 RIP:
[  465.119212]  [<ffffffff802785fc>] __audit_signal_info+0x3c/0x150
[  465.127628] PGD 79f32067 PUD 0
[  465.130772] Oops: 0000 [1] PREEMPT SMP
[  465.134614] CPU 1
[  465.136622] Modules linked in: nfsd exportfs autofs4 binfmt_misc ext2 sbs fan d
ock container battery ac nvram loop evbug evdev thermal psmouse i2c_piix4 processo
r button i2c_core sr_mod cdrom sg shpchp pci_hotplug sd_mod ext3 jbd mbcache ehci_
hcd ohci_hcd uhci_hcd usbcore
[  465.160924] Pid: 3151, comm: sshd Not tainted 2.6.23-rc1 #8
[  465.166465] RIP: 0010:[<ffffffff802785fc>]  [<ffffffff802785fc>] __audit_signal_info+0x3c/0x150
[  465.175128] RSP: 0018:ffff8100731e5be8  EFLAGS: 00010202
[  465.180408] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff8100718b0000
[  465.187503] RDX: 0000000000000001 RSI: ffff810068614000 RDI: 0000000000000002
[  465.194600] RBP: ffff8100731e5bf8 R08: 0000000000000001 R09: 0000000000000000
[  465.201697] R10: 0000000000000001 R11: 0000000000000001 R12: ffff810068614000
[  465.208792] R13: ffff810068614000 R14: 0000000000000001 R15: ffff810074e77000
[  465.215888] FS:  00002b8c2dc90870(0000) GS:ffff810001102380(0000) knlGS:0000000000000000
[  465.223935] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  465.229649] CR2: 0000000000000484 CR3: 0000000037cfc000 CR4: 00000000000006e0
[  465.236745] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  465.243841] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  465.250936] Process sshd (pid: 3151, threadinfo ffff8100731e4000, task ffff8100718b0000)
[  465.258983] Stack:  0000000000000001 0000000000000002 ffff8100731e5c28 ffffffff80247788
[  465.266993]  0000000000200200 ffff810068614218 0000000000000002 ffff810068614000
[  465.274388]  ffff8100731e5c68 ffffffff80248bb6 ffff8100731e5c78 0000000000000246
[  465.281599] Call Trace:
[  465.284215]  [<ffffffff80247788>] check_kill_permission+0x88/0x160
[  465.290362]  [<ffffffff80248bb6>] group_send_sig_info+0x26/0x90
[  465.296249]  [<ffffffff80248eca>] __kill_pgrp_info+0x3a/0x70
[  465.301877]  [<ffffffff80248f37>] kill_pgrp_info+0x37/0x60
[  465.307332]  [<ffffffff80248f78>] kill_pgrp+0x18/0x20
[  465.312355]  [<ffffffff803a31ce>] n_tty_receive_buf+0x76e/0x1010
[  465.318331]  [<ffffffff80423ffc>] sock_aio_read+0x14c/0x160
[  465.323874]  [<ffffffff8025a0d6>] get_lock_stats+0x16/0x60
[  465.329328]  [<ffffffff8025a12e>] put_lock_stats+0xe/0x40
[  465.334696]  [<ffffffff8025a1c3>] lock_release_holdtime+0x63/0x80
[  465.340756]  [<ffffffff802535a9>] add_wait_queue+0x49/0x60
[  465.346213]  [<ffffffff803a537c>] pty_write+0x4c/0x60
[  465.351238]  [<ffffffff803a2935>] write_chan+0x255/0x380
[  465.356521]  [<ffffffff80233f80>] default_wake_function+0x0/0x10
[  465.362496]  [<ffffffff8039fca9>] tty_write+0x199/0x250
[  465.367690]  [<ffffffff803a26e0>] write_chan+0x0/0x380
[  465.372800]  [<ffffffff802ae0a4>] vfs_write+0xe4/0x190
[  465.377910]  [<ffffffff802ae770>] sys_write+0x50/0x90
[  465.382933]  [<ffffffff8020c1be>] system_call+0x7e/0x83
[  465.388131]
[  465.389610]
[  465.389610] Code: 8b 83 84 04 00 00 85 c0 74 53 48 8b 83 48 04 00 00 48 85 c0

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

* [PATCH] audit: fix two bugs in the new execve audit code
  2007-07-27 20:13 ` Steve Grubb
                     ` (2 preceding siblings ...)
  2007-07-27 21:03   ` Alexander Viro
@ 2007-07-27 22:55   ` Peter Zijlstra
  2007-07-27 23:05     ` Andrew Morton
  2007-07-28  2:05     ` Steve Grubb
  3 siblings, 2 replies; 14+ messages in thread
From: Peter Zijlstra @ 2007-07-27 22:55 UTC (permalink / raw)
  To: Steve Grubb, Linus Torvalds, Andrew Morton; +Cc: linux-audit, aaw

On Fri, 2007-07-27 at 16:13 -0400, Steve Grubb wrote:
> Hi,
> 
> I was testing our rawhide kernel and I'm scrolling these errors:
> 
> WARNING: at kernel/auditsc.c:859 audit_log_execve_info() (Not tainted)
> 
> Call Trace:
>  [<ffffffff8106b06f>] audit_log_exit+0x5d7/0x964
>  [<ffffffff81050805>] trace_hardirqs_on+0x12e/0x151
>  [<ffffffff8106b60b>] audit_syscall_exit+0x9b/0x300
>  [<ffffffff8100ee62>] syscall_trace_leave+0x2c/0x87
>  [<ffffffff8100beb1>] int_very_careful+0x3a/0x43
> 

--

copy_from_user() returns the number of bytes not copied, hence 0 is the
expected output.

axi->mm might not be valid anymore when not equal to current->mm, do not
dereference before checking that - thanks to Al for spotting that.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/auditsc.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/auditsc.c
===================================================================
--- linux-2.6.orig/kernel/auditsc.c
+++ linux-2.6/kernel/auditsc.c
@@ -824,12 +824,14 @@ static void audit_log_execve_info(struct
 {
 	int i;
 	long len, ret;
-	const char __user *p = (const char __user *)axi->mm->arg_start;
+	const char __user *p;
 	char *buf;
 
 	if (axi->mm != current->mm)
 		return; /* execve failed, no additional info */
 
+	p = (const char __user *)axi->mm->arg_start;
+
 	for (i = 0; i < axi->argc; i++, p += len) {
 		len = strnlen_user(p, MAX_ARG_STRLEN);
 		/*
@@ -855,7 +857,7 @@ static void audit_log_execve_info(struct
 		 * copied them here, and the mm hasn't been exposed to user-
 		 * space yet.
 		 */
-		if (!ret) {
+		if (ret) {
 			WARN_ON(1);
 			send_sig(SIGKILL, current, 0);
 		}

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

* Re: [PATCH] audit: fix two bugs in the new execve audit code
  2007-07-27 22:55   ` [PATCH] audit: fix two bugs in the new execve audit code Peter Zijlstra
@ 2007-07-27 23:05     ` Andrew Morton
  2007-07-28  2:05     ` Steve Grubb
  1 sibling, 0 replies; 14+ messages in thread
From: Andrew Morton @ 2007-07-27 23:05 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Linus Torvalds, aaw, linux-audit

On Sat, 28 Jul 2007 00:55:18 +0200
Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> -		if (!ret) {
> +		if (ret) {
>  			WARN_ON(1);
>  			send_sig(SIGKILL, current, 0);
>  		}

fwiw, that could now become

	if (WARN_ON(ret))
		send_sig(SIGKILL, current, 0);

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

* Re: [PATCH] audit: fix two bugs in the new execve audit code
  2007-07-27 22:55   ` [PATCH] audit: fix two bugs in the new execve audit code Peter Zijlstra
  2007-07-27 23:05     ` Andrew Morton
@ 2007-07-28  2:05     ` Steve Grubb
  1 sibling, 0 replies; 14+ messages in thread
From: Steve Grubb @ 2007-07-28  2:05 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Andrew Morton, linux-audit, Linus Torvalds, aaw

On Friday 27 July 2007 18:55:18 Peter Zijlstra wrote:
> copy_from_user() returns the number of bytes not copied, hence 0 is the
> expected output.
>
> axi->mm might not be valid anymore when not equal to current->mm, do not
> dereference before checking that - thanks to Al for spotting that.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

This patch tests good applied to the same rawhide kernel source that reported 
the problems.

-Steve

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

end of thread, other threads:[~2007-07-28  2:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-19  8:48 [patch 058/209] audit: rework execve audit akpm
2007-07-27 20:13 ` Steve Grubb
2007-07-27 20:44   ` Peter Zijlstra
2007-07-27 20:57     ` Steve Grubb
2007-07-27 21:55       ` Peter Zijlstra
2007-07-27 22:06         ` Peter Zijlstra
2007-07-27 22:24           ` Peter Zijlstra
2007-07-27 20:50   ` Alexander Viro
2007-07-27 21:03   ` Alexander Viro
2007-07-27 21:11     ` Steve Grubb
2007-07-27 21:21       ` Alexander Viro
2007-07-27 22:55   ` [PATCH] audit: fix two bugs in the new execve audit code Peter Zijlstra
2007-07-27 23:05     ` Andrew Morton
2007-07-28  2:05     ` Steve Grubb

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox