[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

RE: Linux-audit Digest, Vol 18, Issue 31 - Kernel level for RHEL4u2



RHEL4 update 2 is at 2.6.9-22.EL 

-----Original Message-----
From: linux-audit-bounces redhat com
[mailto:linux-audit-bounces redhat com] On Behalf Of
linux-audit-request redhat com
Sent: Thursday, March 30, 2006 3:57 AM
To: linux-audit redhat com
Subject: Linux-audit Digest, Vol 18, Issue 31

Send Linux-audit mailing list submissions to
	linux-audit redhat com

To subscribe or unsubscribe via the World Wide Web, visit
	https://www.redhat.com/mailman/listinfo/linux-audit
or, via email, send a message with subject or body 'help' to
	linux-audit-request redhat com

You can reach the person managing the list at
	linux-audit-owner redhat com

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Linux-audit digest..."


Today's Topics:

   1. Re: Error on stop (Steve Grubb)
   2. [PATCH] change lspp inode auditing (Steve Grubb)
   3. Re: [PATCH] change lspp inode auditing (Stephen Smalley)
   4. Re: [PATCH] change lspp inode auditing (Steve Grubb)
   5. Re: [PATCH] change lspp inode auditing (Serge E. Hallyn)
   6. Re: [PATCH] change lspp inode auditing  (Valdis Kletnieks vt edu)
   7. Re: [PATCH] change lspp inode auditing (Steve Grubb)
   8. auditctl -l bug? (Michael C Thompson)
   9. moving audit_free() up into do_exit() (Alexander Viro)
  10. Re: moving audit_free() up into do_exit() (Steve Grubb)
  11. Re: moving audit_free() up into do_exit() (Alexander Viro)
  12. Re: moving audit_free() up into do_exit() (Alexander Viro)


----------------------------------------------------------------------

Message: 1
Date: Tue, 28 Mar 2006 19:31:30 -0500
From: Steve Grubb <sgrubb redhat com>
Subject: Re: Error on stop
To: linux-audit redhat com
Message-ID: <200603281931 30320 sgrubb redhat com>
Content-Type: text/plain;  charset="utf-8"

On Tuesday 28 March 2006 18:23, Mont Rothstein wrote:
> I am on audit-1.0.12 is this still a bug in this version, and if it
is do I
> need to worry about it?
>
> My output from uname -a is:
>
> Linux rheles4rs1.forayadams.foray.com 2.6.9-11.EL #1 Fri May 20
18:17:57
> EDT 2005 i686 i686 i386 GNU/Linux

I think you need to be on the 2.6.9-.16.EL kernel at a minimum. Offhand
I 
don't remember what's the current RHEL4 kernel, but you need to upgrade

kernels for it to work right.

-Steve



------------------------------

Message: 2
Date: Wed, 29 Mar 2006 13:28:42 -0500
From: Steve Grubb <sgrubb redhat com>
Subject: [PATCH] change lspp inode auditing
To: linux-audit redhat com
Cc: redhat-lspp redhat com
Message-ID: <200603291328 42394 sgrubb redhat com>
Content-Type: text/plain;  charset="us-ascii"

Hi,

This is a first draft patch to change the auditing of inodes for lspp.
Previously, we were gathering the context instead of the sid. Now in
this patch, 
we gather just the sid and convert to context only if an audit event is
being 
output. This patch makes no effort to account for policy_load. It also
inserts
some functions that are likely going upstream via Se Linux kernel
people. So,
that will need to be resolved before this patch is final. In any event
its
good enough to test with. This patch brings the performance hit from
146% down to 11%. We need a similar patch for IPC syscall auditing.

-Steve


diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
--- linux-2.6.16.x86_64.orig/include/linux/selinux.h	2006-03-29
10:40:42.000000000 -0500
+++ linux-2.6.16.x86_64/include/linux/selinux.h	2006-03-29
10:27:06.000000000 -0500
@@ -13,6 +13,8 @@
 #ifndef _LINUX_SELINUX_H
 #define _LINUX_SELINUX_H
 
+#include <linux/fs.h>
+
 struct selinux_audit_rule;
 struct audit_context;
 
@@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
  */
 void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
 
+/**
+ *     selinux_ctxid_to_string - map a security context ID to a string
+ *     @ctxid: security context ID to be converted.
+ *     @ctx: address of context string to be returned
+ *     @ctxlen: length of returned context string.
+ *
+ *     Returns 0 if successful, -errno if not.  On success, the
context
+ *     string will be allocated internally, and the caller must call
+ *     kfree() on it after use.
+ */
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
+
+/**
+ *     selinux_get_inode_sid - get the inode's security context ID
+ *     @inode: inode structure to get the sid from.
+ *
+ *     Returns the sid if successful and 0 if unset
+ */
+u32 selinux_get_inode_sid(const struct inode *inode);
+
 #else
 
 static inline int selinux_audit_rule_init(u32 field, u32 op,
@@ -107,6 +129,18 @@ static inline void selinux_task_ctxid(st
 	*ctxid = 0;
 }
 
+static inline int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32
*ctxlen)
+{
+       *ctx = NULL;
+       *ctxlen = 0;
+       return 0;
+}
+
+static inline u32 selinux_get_inode_sid(const struct inode *inode)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_SECURITY_SELINUX */
 
 #endif /* _LINUX_SELINUX_H */
diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c
linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c	2006-03-29
10:40:48.000000000 -0500
+++ linux-2.6.16.x86_64/kernel/auditsc.c	2006-03-29
10:26:45.000000000 -0500
@@ -90,7 +90,7 @@ struct audit_names {
 	uid_t		uid;
 	gid_t		gid;
 	dev_t		rdev;
-	char		*ctx;
+	u32		osid;
 };
 
 struct audit_aux_data {
@@ -435,9 +435,6 @@ static inline void audit_free_names(stru
 #endif
 
 	for (i = 0; i < context->name_count; i++) {
-		char *p = context->names[i].ctx;
-		context->names[i].ctx = NULL;
-		kfree(p);
 		if (context->names[i].name)
 			__putname(context->names[i].name);
 	}
@@ -729,9 +726,24 @@ static void audit_log_exit(struct audit_
 					 context->names[i].gid, 
 					 MAJOR(context->names[i].rdev),

 
MINOR(context->names[i].rdev));
-		if (context->names[i].ctx) {
-			audit_log_format(ab, " obj=%s",
-					context->names[i].ctx);
+		if (context->names[i].osid != 0) {
+			char *ctx = NULL;
+			int len = 0;
+			if (selinux_ctxid_to_string(
+				context->names[i].osid, &ctx, &len) ==
0) {
+				ctx = kmalloc(len, gfp_mask);
+				if (ctx) {
+					selinux_ctxid_to_string(
+		                                context->names[i].osid,
+						&ctx, &len);
+				}
+			}
+			if (ctx)
+				audit_log_format(ab, " obj=%s", ctx);
+			else
+				audit_log_format(ab, " obj=%u",
+
context->names[i].osid);
+			kfree(ctx);
 		}
 
 		audit_log_end(ab);
@@ -983,37 +995,10 @@ void audit_putname(const char *name)
 void audit_inode_context(int idx, const struct inode *inode)
 {
 	struct audit_context *context = current->audit_context;
-	const char *suffix = security_inode_xattr_getsuffix();
-	char *ctx = NULL;
-	int len = 0;
-
-	if (!suffix)
-		goto ret;
-
-	len = security_inode_getsecurity(inode, suffix, NULL, 0, 0);
-	if (len == -EOPNOTSUPP)
-		goto ret;
-	if (len < 0) 
-		goto error_path;
-
-	ctx = kmalloc(len, GFP_KERNEL);
-	if (!ctx) 
-		goto error_path;
-
-	len = security_inode_getsecurity(inode, suffix, ctx, len, 0);
-	if (len < 0)
-		goto error_path;
-
-	kfree(context->names[idx].ctx);
-	context->names[idx].ctx = ctx;
-	goto ret;
-
-error_path:
-	if (ctx)
-		kfree(ctx);
-	audit_panic("error in audit_inode_context");
-ret:
-	return;
+	if (security_inode_xattr_getsuffix())
+		context->names[idx].osid =
selinux_get_inode_sid(inode);
+	else
+		context->names[idx].osid = 0;
 }
 
 
diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
--- linux-2.6.16.x86_64.orig/security/selinux/exports.c	2006-03-29
10:40:51.000000000 -0500
+++ linux-2.6.16.x86_64/security/selinux/exports.c	2006-03-29
10:26:45.000000000 -0500
@@ -26,3 +26,24 @@ void selinux_task_ctxid(struct task_stru
 	else
 		*ctxid = 0;
 }
+
+extern int ss_initialized;
+
+int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen)
+{
+       if (ss_initialized)
+               return security_sid_to_context(ctxid, ctx, ctxlen);
+       else {
+               *ctx = NULL;
+               *ctxlen = 0;
+       }
+
+       return 0;
+}
+
+u32 selinux_get_inode_sid(const struct inode *inode)
+{
+	struct inode_security_struct *isec = inode->i_security;
+	return isec->sid;
+}
+



------------------------------

Message: 3
Date: Wed, 29 Mar 2006 14:01:22 -0500
From: Stephen Smalley <sds tycho nsa gov>
Subject: Re: [PATCH] change lspp inode auditing
To: Steve Grubb <sgrubb redhat com>
Cc: redhat-lspp redhat com, linux-audit redhat com,	James Morris
	<jmorris namei org>
Message-ID: <1143658882 24555 59 camel moss-spartans epoch ncsc mil>
Content-Type: text/plain

On Wed, 2006-03-29 at 13:28 -0500, Steve Grubb wrote:
> Hi,
> 
> This is a first draft patch to change the auditing of inodes for
lspp.
> Previously, we were gathering the context instead of the sid. Now in
this patch, 
> we gather just the sid and convert to context only if an audit event
is being 
> output. This patch makes no effort to account for policy_load. It
also inserts
> some functions that are likely going upstream via Se Linux kernel
people. So,
> that will need to be resolved before this patch is final. In any
event its
> good enough to test with. This patch brings the performance hit from
> 146% down to 11%. We need a similar patch for IPC syscall auditing.

Not that I disagree with this change in approach, but I think that when
it has come up in the past, there has been concern expressed about the
fact that we could end up not being able to generate the context from
the SID when the audit record is being emitted (due to OOM condition),
and the operation has already occurred at that point.  Of course, there
are also other potential failure cases at the point, so I'm not sure it
is crucial, as long as audit_panic is called as appropriate.  Just
wanted to make sure that this point was understood by everyone.  But I
agree that pre-allocating the contexts is insane.

> diff -urp linux-2.6.16.x86_64.orig/include/linux/selinux.h
linux-2.6.16.x86_64/include/linux/selinux.h
> --- linux-2.6.16.x86_64.orig/include/linux/selinux.h	2006-03-29
10:40:42.000000000 -0500
> +++ linux-2.6.16.x86_64/include/linux/selinux.h	2006-03-29
10:27:06.000000000 -0500
> @@ -13,6 +13,8 @@
>  #ifndef _LINUX_SELINUX_H
>  #define _LINUX_SELINUX_H
>  
> +#include <linux/fs.h>

Just put an empty decl for struct inode here, to avoid header
inter-dependencies:
	struct inode;

> @@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
>   */
>  void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
>  
> +/**
> + *     selinux_ctxid_to_string - map a security context ID to a
string
> + *     @ctxid: security context ID to be converted.
> + *     @ctx: address of context string to be returned
> + *     @ctxlen: length of returned context string.
> + *
> + *     Returns 0 if successful, -errno if not.  On success, the
context
> + *     string will be allocated internally, and the caller must call
> + *     kfree() on it after use.
> + */
> +int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);

Didn't Tim's patch for saving and auditing the netlink sender
SID/context have a similar interface, based on James' proposed API for
iptables?  Just need to make sure that we settle on a single interface
used by them all.  From later comments below, I think we'll want a
gfp_mask provided and I'm not sure we need the *ctxlen at all, as
SELinux handles the allocation internally.

> +/**
> + *     selinux_get_inode_sid - get the inode's security context ID
> + *     @inode: inode structure to get the sid from.
> + *
> + *     Returns the sid if successful and 0 if unset
> + */
> +u32 selinux_get_inode_sid(const struct inode *inode);

I'd favor returning an int (0 == success, -errno for failure) and
providing the SID via pointer arg like other interfaces.  Or if there
are no possible error cases, make it void, but still supply the SID via
argument.

> diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c
linux-2.6.16.x86_64/kernel/auditsc.c
> --- linux-2.6.16.x86_64.orig/kernel/auditsc.c	2006-03-29
10:40:48.000000000 -0500
> +++ linux-2.6.16.x86_64/kernel/auditsc.c	2006-03-29
10:26:45.000000000 -0500
> @@ -729,9 +726,24 @@ static void audit_log_exit(struct audit_
>  					 context->names[i].gid, 
>  					 MAJOR(context->names[i].rdev),

>
MINOR(context->names[i].rdev));
> -		if (context->names[i].ctx) {
> -			audit_log_format(ab, " obj=%s",
> -					context->names[i].ctx);
> +		if (context->names[i].osid != 0) {
> +			char *ctx = NULL;
> +			int len = 0;
> +			if (selinux_ctxid_to_string(
> +				context->names[i].osid, &ctx, &len) ==
0) {
> +				ctx = kmalloc(len, gfp_mask);
> +				if (ctx) {
> +					selinux_ctxid_to_string(
> +		                                context->names[i].osid,
> +						&ctx, &len);
> +				}
> +			}

Unless I'm confused (quite possible ;), the above sequence shouldn't be
necessary and will actually leak the allocated buffer because SELinux
will overwrite the pointer with its own.  The SELinux internal
functions
(e.g. security_sid_to_context) handle the allocation of a context
buffer
to the right size and set *ctx to it, so the caller never needs to play
this game.   Some of the hook interfaces unfortunately require the
caller to guess and provide a buffer that they allocate, but I don't
think we want to continue that trend.  SELinux should just set *ctx to
the context buffer it allocates and you are done.  You should likely
pass the gfp_mask down into the SELinux interface and propagate it down
to the internal code so that we can conform with whatever the caller
needs.

> diff -urp linux-2.6.16.x86_64.orig/security/selinux/exports.c
linux-2.6.16.x86_64/security/selinux/exports.c
> --- linux-2.6.16.x86_64.orig/security/selinux/exports.c
2006-03-29 10:40:51.000000000 -0500
> +++ linux-2.6.16.x86_64/security/selinux/exports.c	2006-03-29
10:26:45.000000000 -0500
> +u32 selinux_get_inode_sid(const struct inode *inode)
> +{
> +	struct inode_security_struct *isec = inode->i_security;
> +	return isec->sid;
> +}

I think you need to check for selinux_enabled here, c.f. Darrel's
patches for audit-by-context.  Keep in mind that SELinux can be runtime
disabled by init (if SELINUX=disabled in /etc/selinux/config).

-- 
Stephen Smalley
National Security Agency



------------------------------

Message: 4
Date: Wed, 29 Mar 2006 14:15:33 -0500
From: Steve Grubb <sgrubb redhat com>
Subject: Re: [PATCH] change lspp inode auditing
To: sds tycho nsa gov
Cc: redhat-lspp redhat com, linux-audit redhat com,	James Morris
	<jmorris namei org>
Message-ID: <200603291415 33608 sgrubb redhat com>
Content-Type: text/plain;  charset="utf-8"

On Wednesday 29 March 2006 14:01, Stephen Smalley wrote:
>> This patch brings the performance hit from 146% down to 11%. We need
a
>> similar patch for IPC syscall auditing. 
>
> Not that I disagree with this change in approach, but I think that
when
> it has come up in the past, there has been concern expressed about
the
> fact that we could end up not being able to generate the context from
> the SID when the audit record is being emitted (due to OOM
condition),
> and the operation has already occurred at that point.

In that case, the patch writes out the sid number. Given a sid, is
there a way 
to find it in the policy on disk? If not, that might be useful to have.

> Of course, there are also other potential failure cases at the point,
so I'm
> not sure it is crucial, as long as audit_panic is called as
> appropriate. 

If we record the sid number, do we really need to call audit_panic?

> > @@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
> >   */
> >  void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
> >  
> > +/**
> > + *     selinux_ctxid_to_string - map a security context ID to a
string
> > + *     @ctxid: security context ID to be converted.
> > + *     @ctx: address of context string to be returned
> > + *     @ctxlen: length of returned context string.
> > + *
> > + *     Returns 0 if successful, -errno if not.  On success, the
context
> > + *     string will be allocated internally, and the caller must
call
> > + *     kfree() on it after use.
> > + */
> > +int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
>
> Didn't Tim's patch for saving and auditing the netlink sender
> SID/context have a similar interface, based on James' proposed API
for
> iptables?

Yes, I copy and pasted and changed the name based on a suggestion from
Darrel. 
What is the status of that API? Did it go into 2.6.17 tree? I'd like to
code 
to that API if it were available.

> > +             if (context->names[i].osid != 0) {
> > +                     char *ctx = NULL;
> > +                     int len = 0;
> > +                     if
(selinux_ctxid_to_string(
> >
+                             context->nam
es[i].osid, &ctx, &len) == 0) {
> > +                             ctx =
kmalloc(len, gfp_mask);
> > +                             if (ctx)
{
> >
+                                   
  selinux_ctxid_to_string(
> > +                                       
     context->names[i].osid,
> >
+                                   
          &ctx, &len);
> > +                             }
> > +                     }
>
> Unless I'm confused (quite possible ;), the above sequence shouldn't
be
> necessary and will actually leak the allocated buffer because SELinux
> will overwrite the pointer with its own.

OK, will look into this.

> Some of the hook interfaces unfortunately require the caller to guess
and
> provide a buffer that they allocate, but I don't think we want to
continue
> that trend.

Agreed, that was messy.

I'll make changes as you suggested and we can try this again. Is there
a place 
I can grab James' iptables SE Linux interface to patch the lspp kernel
with? 
I'd like to use that if its accepted/done. It'll make merging Tim's
patch 
easier.

-Steve



------------------------------

Message: 5
Date: Wed, 29 Mar 2006 13:18:35 -0600
From: "Serge E. Hallyn" <serue us ibm com>
Subject: Re: [PATCH] change lspp inode auditing
To: Stephen Smalley <sds tycho nsa gov>
Cc: redhat-lspp redhat com, James Morris <jmorris namei org>,
	linux-audit redhat com
Message-ID: <20060329191835 GB30125 sergelap austin ibm com>
Content-Type: text/plain; charset=us-ascii

Quoting Stephen Smalley (sds tycho nsa gov):
> On Wed, 2006-03-29 at 13:28 -0500, Steve Grubb wrote:
> > Hi,
> > 
> > This is a first draft patch to change the auditing of inodes for
lspp.
> > Previously, we were gathering the context instead of the sid. Now
in this patch, 
> > we gather just the sid and convert to context only if an audit
event is being 
> > output. This patch makes no effort to account for policy_load. It
also inserts
> > some functions that are likely going upstream via Se Linux kernel
people. So,
> > that will need to be resolved before this patch is final. In any
event its
> > good enough to test with. This patch brings the performance hit
from
> > 146% down to 11%. We need a similar patch for IPC syscall auditing.
> 
> Not that I disagree with this change in approach, but I think that
when
> it has come up in the past, there has been concern expressed about
the
> fact that we could end up not being able to generate the context from
> the SID when the audit record is being emitted (due to OOM
condition),
> and the operation has already occurred at that point.  Of course,
there
> are also other potential failure cases at the point, so I'm not sure
it
> is crucial, as long as audit_panic is called as appropriate.  Just
> wanted to make sure that this point was understood by everyone.  But
I
> agree that pre-allocating the contexts is insane.

Maybe this is a silly idea...  but what about just somehow hashing on
(sid,policy_version), where uint policy_version is incremented on each
selinux policy load?

The audit code would fill in an entry for
au_ctx_hash(sid,policy_version),
if it isn't already filled in, when the context would previously have
been
preallocated.  But it stores (sid, policy_version) in the audit record,
and grabs the value from the table when it's time to actually log the
entry, i.e. where Steve's current patch fills in the string.

I guess whether this is worth it depends on how likely we are to lose
information with this current patch on a live system.

Anyway, just a thought.

-serge



------------------------------

Message: 6
Date: Wed, 29 Mar 2006 14:34:19 -0500
From: Valdis Kletnieks vt edu
Subject: Re: [PATCH] change lspp inode auditing 
To: Steve Grubb <sgrubb redhat com>
Cc: redhat-lspp redhat com, linux-audit redhat com,	James Morris
	<jmorris namei org>
Message-ID: <200603291934 k2TJYJNu029217 turing-police cc vt edu>
Content-Type: text/plain; charset="iso-8859-1"

On Wed, 29 Mar 2006 14:15:33 EST, Steve Grubb said:
> On Wednesday 29 March 2006 14:01, Stephen Smalley wrote:
> >> This patch brings the performance hit from 146% down to 11%. We
need a
> >> similar patch for IPC syscall auditing. 
> >
> > Not that I disagree with this change in approach, but I think that
when
> > it has come up in the past, there has been concern expressed about
the
> > fact that we could end up not being able to generate the context
from
> > the SID when the audit record is being emitted (due to OOM
condition),
> > and the operation has already occurred at that point.
> 
> In that case, the patch writes out the sid number. Given a sid, is
there a way 
> to find it in the policy on disk? If not, that might be useful to
have.

The problem is that by the time you go to snarf it out of the policy on
disk,
it may no longer match the policy in effect at the time of the record
generation.

The hole probably isn't *that* bad if auditd is doing the grovelling.
It's almost
certainly an issue if ausearch is doing the correlation after the
fact....

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
Url :
https://www.redhat.com/archives/linux-audit/attachments/20060329/293164
c1/attachment.bin

------------------------------

Message: 7
Date: Wed, 29 Mar 2006 14:44:10 -0500
From: Steve Grubb <sgrubb redhat com>
Subject: Re: [PATCH] change lspp inode auditing
To: Valdis Kletnieks vt edu
Cc: redhat-lspp redhat com, linux-audit redhat com,	James Morris
	<jmorris namei org>
Message-ID: <200603291444 10205 sgrubb redhat com>
Content-Type: text/plain;  charset="utf-8"

On Wednesday 29 March 2006 14:34, Valdis Kletnieks vt edu wrote:
> > In that case, the patch writes out the sid number. Given a sid, is
there
> > a way to find it in the policy on disk? If not, that might be
useful to
> > have.
>
> The problem is that by the time you go to snarf it out of the policy
on
> disk, it may no longer match the policy in effect at the time of the
record
> generation.

That should be handled by site configuration control. Assuming that
they are 
careful to keep old policy around...can it be correlated?

> The hole probably isn't *that* bad if auditd is doing the grovelling.

Auditd has no time to do any correlation. This would have to be done 
post-mortem just like uid conversion is done. I think this is an
exceptional 
condition and just want to make sure we can close the loop manually if
this 
ever happened.

-Steve



------------------------------

Message: 8
Date: Wed, 29 Mar 2006 16:02:42 -0600
From: Michael C Thompson <mcthomps us ibm com>
Subject: auditctl -l bug?
To: Steve Grubb <sgrubb redhat com>
Cc: linux-audit redhat com
Message-ID:
	
<OF5387E22F 1FC41932-ON87257140 0078C384-86257140 0078B373 us ibm com>
Content-Type: text/plain; charset="us-ascii"

linux-audit-bounces redhat com wrote on 03/28/2006 06:31:30 PM:

> On Tuesday 28 March 2006 18:23, Mont Rothstein wrote:
> > I am on audit-1.0.12 is this still a bug in this version, and if it
is 
do I
> > need to worry about it?
> >
> > My output from uname -a is:
> >
> > Linux rheles4rs1.forayadams.foray.com 2.6.9-11.EL #1 Fri May 20 
18:17:57
> > EDT 2005 i686 i686 i386 GNU/Linux
> 
> I think you need to be on the 2.6.9-.16.EL kernel at a minimum.
Offhand 
I 
> don't remember what's the current RHEL4 kernel, but you need to
upgrade 
> kernels for it to work right.
> 
> -Steve

Hey Steve,

With a yum upgrade against FC5, using "auditctl -l" yields:
# auditctl -l
Error sending rule list request (Operation not permitted)
File system watches not supportedctl -l for the first time yeilds:

After this, any subsequent "auditctl -l" commands will yeild:
# auditctl -l
No rules
File system watches not supported


Am I the only one seeing this, and if so, any suggestions on what I can
do 
to track this down?

Thanks,
Mike
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
https://www.redhat.com/archives/linux-audit/attachments/20060329/1ac17a
2f/attachment.html

------------------------------

Message: 9
Date: Wed, 29 Mar 2006 17:03:58 -0500
From: Alexander Viro <aviro redhat com>
Subject: moving audit_free() up into do_exit()
To: linux-audit redhat com
Cc: akpm osdl org
Message-ID: <20060329220358 GN1727 devserv devel redhat com>
Content-Type: text/plain; charset=us-ascii

        If we want to get full information on syscalls that kill the
caller
(exit, exit_group, anything oopsing), we really ought to move
audit_free()
into do_exit(), just before the exit_mm() call there.

        Impact: we'll generate records for such syscall when it's
called
(and gets to the point of no return), not when the resulting zombie
gets
reaped.  If anyone has objections to that, yell _NOW_.

        What we get is, among other things, accurate tty=, exe= and
subj=
in such records.  We also get much simpler locking, since now _all_
access to ->audit_context is done by process itself in a
process-synchronous
context.  We don't have to bother with atomic allocations on that path
either.

        NOTE: it does change the moment when record is generated and if
something in userland depends on having it postponed until the time
when
zombie gets reaped, we will have to change such userland code.  I doubt
we have any such place, but that definitely needs an ACK from userland
side of things.



------------------------------

Message: 10
Date: Wed, 29 Mar 2006 17:21:34 -0500
From: Steve Grubb <sgrubb redhat com>
Subject: Re: moving audit_free() up into do_exit()
To: linux-audit redhat com
Cc: akpm osdl org
Message-ID: <200603291721 34472 sgrubb redhat com>
Content-Type: text/plain;  charset="iso-8859-1"

On Wednesday 29 March 2006 17:03, Alexander Viro wrote:
> NOTE: it does change the moment when record is generated and if
> something in userland depends on having it postponed until the time
when
> zombie gets reaped, we will have to change such userland code.  I
doubt
> we have any such place, but that definitely needs an ACK from
userland
> side of things.

User land - e.g. auditd - does not have any dependency. It is a change
that 
people doing EAL documentation may want to be aware of.

Thanks,
-Steve



------------------------------

Message: 11
Date: Thu, 30 Mar 2006 03:54:53 -0500
From: Alexander Viro <aviro redhat com>
Subject: Re: moving audit_free() up into do_exit()
To: Steve Grubb <sgrubb redhat com>
Cc: akpm osdl org, linux-audit redhat com
Message-ID: <20060330085453 GP1727 devserv devel redhat com>
Content-Type: text/plain; charset=us-ascii

On Wed, Mar 29, 2006 at 05:21:34PM -0500, Steve Grubb wrote:
> On Wednesday 29 March 2006 17:03, Alexander Viro wrote:
> > NOTE: it does change the moment when record is generated and if
> > something in userland depends on having it postponed until the time
when
> > zombie gets reaped, we will have to change such userland code. ?I
doubt
> > we have any such place, but that definitely needs an ACK from
userland
> > side of things.
> 
> User land - e.g. auditd - does not have any dependency. It is a
change that 
> people doing EAL documentation may want to be aware of.

OK, preliminary patches attached; the first one is minimal "take care
of
deadlocks", hopefully suitable for 2.6.16.2.  The second is incremental
to that (actually, a combination of several smaller steps from git
tree)
doing the audit_free() move and cleanups.  Warning: both are absolutely
untested.

Question: do we have agreed upon variants of "Inotify kernel API" and
"filesystem location based auditing" patches against the current
mainline
(or FC, no difference in that respect) tree?



------------------------------

Message: 12
Date: Thu, 30 Mar 2006 03:56:38 -0500
From: Alexander Viro <aviro redhat com>
Subject: Re: moving audit_free() up into do_exit()
To: linux-audit redhat com
Cc: akpm osdl org
Message-ID: <20060330085638 GQ1727 devserv devel redhat com>
Content-Type: text/plain; charset="us-ascii"

On Thu, Mar 30, 2006 at 03:54:53AM -0500, Alexander Viro wrote:
> OK, preliminary patches attached; the first one is minimal "take care
of

Gah...  Attached to this followup; my apologies.
-------------- next part --------------
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 7f160df..4052f0a 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -536,13 +536,13 @@ error_path:
 	return;
 }
 
-static void audit_log_task_info(struct audit_buffer *ab, gfp_t
gfp_mask)
+static void audit_log_task_info(struct audit_buffer *ab, struct
task_struct *tsk, gfp_t gfp_mask)
 {
-	char name[sizeof(current->comm)];
-	struct mm_struct *mm = current->mm;
+	char name[sizeof(tsk->comm)];
+	struct mm_struct *mm = tsk->mm;
 	struct vm_area_struct *vma;
 
-	get_task_comm(name, current);
+	get_task_comm(name, tsk);
 	audit_log_format(ab, " comm=");
 	audit_log_untrustedstring(ab, name);
 
@@ -551,7 +551,7 @@ static void audit_log_task_info(struct a
 
 	/*
 	 * this is brittle; all callers that pass GFP_ATOMIC will have
-	 * NULL current->mm and we won't get here.
+	 * NULL tsk->mm and we won't get here.
 	 */
 	down_read(&mm->mmap_sem);
 	vma = mm->mmap;
@@ -569,7 +569,7 @@ static void audit_log_task_info(struct a
 	audit_log_task_context(ab, gfp_mask);
 }
 
-static void audit_log_exit(struct audit_context *context, gfp_t
gfp_mask)
+static void audit_log_exit(struct audit_context *context, struct
task_struct *tsk, gfp_t gfp_mask)
 {
 	int i;
 	struct audit_buffer *ab;
@@ -587,8 +587,8 @@ static void audit_log_exit(struct audit_
 		audit_log_format(ab, " success=%s exit=%ld", 
 
(context->return_valid==AUDITSC_SUCCESS)?"yes":"no",
 				 context->return_code);
-	if (current->signal->tty && current->signal->tty->name)
-		tty = current->signal->tty->name;
+	if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
+		tty = tsk->signal->tty->name;
 	else
 		tty = "(none)";
 	audit_log_format(ab,
@@ -720,7 +720,7 @@ void audit_free(struct task_struct *tsk)
 	 * We use GFP_ATOMIC here because we might be doing this 
 	 * in the context of the idle thread */
 	if (context->in_syscall && context->auditable)
-		audit_log_exit(context, GFP_ATOMIC);
+		audit_log_exit(context, tsk, GFP_ATOMIC);
 
 	audit_free_context(context);
 }
@@ -839,7 +839,7 @@ void audit_syscall_exit(struct task_stru
 		goto out;
 
 	if (context->in_syscall && context->auditable)
-		audit_log_exit(context, GFP_KERNEL);
+		audit_log_exit(context, tsk, GFP_KERNEL);
 
 	context->in_syscall = 0;
 	context->auditable  = 0;
-------------- next part --------------
diff --git a/arch/i386/kernel/ptrace.c b/arch/i386/kernel/ptrace.c
index 506462e..fd7eaf7 100644
--- a/arch/i386/kernel/ptrace.c
+++ b/arch/i386/kernel/ptrace.c
@@ -671,7 +671,7 @@ int do_syscall_trace(struct pt_regs *reg
 
 	if (unlikely(current->audit_context)) {
 		if (entryexit)
-			audit_syscall_exit(current,
AUDITSC_RESULT(regs->eax),
+			audit_syscall_exit(AUDITSC_RESULT(regs->eax),
 						regs->eax);
 		/* Debug traps, when using PTRACE_SINGLESTEP, must be
sent only
 		 * on the syscall exit path. Normally, when
TIF_SYSCALL_AUDIT is
@@ -720,14 +720,13 @@ int do_syscall_trace(struct pt_regs *reg
 	ret = is_sysemu;
 out:
 	if (unlikely(current->audit_context) && !entryexit)
-		audit_syscall_entry(current, AUDIT_ARCH_I386,
regs->orig_eax,
+		audit_syscall_entry(AUDIT_ARCH_I386, regs->orig_eax,
 				    regs->ebx, regs->ecx, regs->edx,
regs->esi);
 	if (ret == 0)
 		return 0;
 
 	regs->orig_eax = -1; /* force skip of syscall restarting */
 	if (unlikely(current->audit_context))
-		audit_syscall_exit(current, AUDITSC_RESULT(regs->eax),
-				regs->eax);
+		audit_syscall_exit(AUDITSC_RESULT(regs->eax),
regs->eax);
 	return 1;
 }
diff --git a/arch/i386/kernel/vm86.c b/arch/i386/kernel/vm86.c
index aee14fa..00e0118 100644
--- a/arch/i386/kernel/vm86.c
+++ b/arch/i386/kernel/vm86.c
@@ -312,7 +312,7 @@ static void do_sys_vm86(struct kernel_vm
 
 	/*call audit_syscall_exit since we do not exit via the normal
paths */
 	if (unlikely(current->audit_context))
-		audit_syscall_exit(current, AUDITSC_RESULT(eax), eax);
+		audit_syscall_exit(AUDITSC_RESULT(eax), eax);
 
 	__asm__ __volatile__(
 		"movl %0,%%esp\n\t"
diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 9887c87..e61e15e 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1644,7 +1644,7 @@ syscall_trace_enter (long arg0, long arg
 			arch = AUDIT_ARCH_IA64;
 		}
 
-		audit_syscall_entry(current, arch, syscall, arg0, arg1,
arg2, arg3);
+		audit_syscall_entry(arch, syscall, arg0, arg1, arg2,
arg3);
 	}
 
 }
@@ -1662,7 +1662,7 @@ syscall_trace_leave (long arg0, long arg
 
 		if (success != AUDITSC_SUCCESS)
 			result = -result;
-		audit_syscall_exit(current, success, result);
+		audit_syscall_exit(success, result);
 	}
 
 	if (test_thread_flag(TIF_SYSCALL_TRACE)
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index f838b36..26ab8a9 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -469,7 +469,7 @@ static inline int audit_arch(void)
 asmlinkage void do_syscall_trace(struct pt_regs *regs, int entryexit)
 {
 	if (unlikely(current->audit_context) && entryexit)
-		audit_syscall_exit(current,
AUDITSC_RESULT(regs->regs[2]),
+		audit_syscall_exit(AUDITSC_RESULT(regs->regs[2]),
 		                   regs->regs[2]);
 
 	if (!(current->ptrace & PT_PTRACED))
@@ -493,7 +493,7 @@ asmlinkage void do_syscall_trace(struct 
 	}
  out:
 	if (unlikely(current->audit_context) && !entryexit)
-		audit_syscall_entry(current, audit_arch(),
regs->regs[2],
+		audit_syscall_entry(audit_arch(), regs->regs[2],
 				    regs->regs[4], regs->regs[5],
 				    regs->regs[6], regs->regs[7]);
 }
diff --git a/arch/powerpc/kernel/ptrace.c
b/arch/powerpc/kernel/ptrace.c
index bcb8357..4a677d1 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -538,7 +538,7 @@ void do_syscall_trace_enter(struct pt_re
 		do_syscall_trace();
 
 	if (unlikely(current->audit_context))
-		audit_syscall_entry(current,
+		audit_syscall_entry(
 #ifdef CONFIG_PPC32
 				    AUDIT_ARCH_PPC,
 #else
@@ -556,8 +556,7 @@ void do_syscall_trace_leave(struct pt_re
 #endif
 
 	if (unlikely(current->audit_context))
-		audit_syscall_exit(current,
-
(regs->ccr&0x1000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
+
audit_syscall_exit((regs->ccr&0x1000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
 				   regs->result);
 
 	if ((test_thread_flag(TIF_SYSCALL_TRACE)
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 37dfe33..8f36504 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -734,7 +734,7 @@ asmlinkage void
 syscall_trace(struct pt_regs *regs, int entryexit)
 {
 	if (unlikely(current->audit_context) && entryexit)
-		audit_syscall_exit(current,
AUDITSC_RESULT(regs->gprs[2]), regs->gprs[2]);
+		audit_syscall_exit(AUDITSC_RESULT(regs->gprs[2]),
regs->gprs[2]);
 
 	if (!test_thread_flag(TIF_SYSCALL_TRACE))
 		goto out;
@@ -761,8 +761,7 @@ syscall_trace(struct pt_regs *regs, int 
 	}
  out:
 	if (unlikely(current->audit_context) && !entryexit)
-		audit_syscall_entry(current, 
-
test_thread_flag(TIF_31BIT)?AUDIT_ARCH_S390:AUDIT_ARCH_S390X,
+
audit_syscall_entry(test_thread_flag(TIF_31BIT)?AUDIT_ARCH_S390:AUDIT_A
RCH_S390X,
 				    regs->gprs[2], regs->orig_gpr2,
regs->gprs[3],
 				    regs->gprs[4], regs->gprs[5]);
 }
diff --git a/arch/sparc64/kernel/ptrace.c
b/arch/sparc64/kernel/ptrace.c
index eb93e9c..bd54daf 100644
--- a/arch/sparc64/kernel/ptrace.c
+++ b/arch/sparc64/kernel/ptrace.c
@@ -630,7 +630,7 @@ asmlinkage void syscall_trace(struct pt_
 		if (unlikely(tstate & (TSTATE_XCARRY | TSTATE_ICARRY)))
 			result = AUDITSC_FAILURE;
 
-		audit_syscall_exit(current, result,
regs->u_regs[UREG_I0]);
+		audit_syscall_exit(result, regs->u_regs[UREG_I0]);
 	}
 
 	if (!(current->ptrace & PT_PTRACED))
@@ -654,8 +654,7 @@ asmlinkage void syscall_trace(struct pt_
 
 out:
 	if (unlikely(current->audit_context) && !syscall_exit_p)
-		audit_syscall_entry(current,
-				    (test_thread_flag(TIF_32BIT) ?
+		audit_syscall_entry((test_thread_flag(TIF_32BIT) ?
 				     AUDIT_ARCH_SPARC :
 				     AUDIT_ARCH_SPARC64),
 				    regs->u_regs[UREG_G1],
diff --git a/arch/um/kernel/ptrace.c b/arch/um/kernel/ptrace.c
index 98e0939..139c3ac 100644
--- a/arch/um/kernel/ptrace.c
+++ b/arch/um/kernel/ptrace.c
@@ -269,15 +269,13 @@ void syscall_trace(union uml_pt_regs *re
 
 	if (unlikely(current->audit_context)) {
 		if (!entryexit)
-			audit_syscall_entry(current,
-                                            HOST_AUDIT_ARCH,
+			audit_syscall_entry(HOST_AUDIT_ARCH,
 					    UPT_SYSCALL_NR(regs),
 					    UPT_SYSCALL_ARG1(regs),
 					    UPT_SYSCALL_ARG2(regs),
 					    UPT_SYSCALL_ARG3(regs),
 					    UPT_SYSCALL_ARG4(regs));
-		else audit_syscall_exit(current,
-
AUDITSC_RESULT(UPT_SYSCALL_RET(regs)),
+		else
audit_syscall_exit(AUDITSC_RESULT(UPT_SYSCALL_RET(regs)),
                                         UPT_SYSCALL_RET(regs));
 	}
 
diff --git a/arch/x86_64/kernel/ptrace.c b/arch/x86_64/kernel/ptrace.c
index d44b2c1..5ef7aae 100644
--- a/arch/x86_64/kernel/ptrace.c
+++ b/arch/x86_64/kernel/ptrace.c
@@ -605,12 +605,12 @@ asmlinkage void syscall_trace_enter(stru
 
 	if (unlikely(current->audit_context)) {
 		if (test_thread_flag(TIF_IA32)) {
-			audit_syscall_entry(current, AUDIT_ARCH_I386,
+			audit_syscall_entry(AUDIT_ARCH_I386,
 					    regs->orig_rax,
 					    regs->rbx, regs->rcx,
 					    regs->rdx, regs->rsi);
 		} else {
-			audit_syscall_entry(current, AUDIT_ARCH_X86_64,
+			audit_syscall_entry(AUDIT_ARCH_X86_64,
 					    regs->orig_rax,
 					    regs->rdi, regs->rsi,
 					    regs->rdx, regs->r10);
@@ -621,7 +621,7 @@ asmlinkage void syscall_trace_enter(stru
 asmlinkage void syscall_trace_leave(struct pt_regs *regs)
 {
 	if (unlikely(current->audit_context))
-		audit_syscall_exit(current, AUDITSC_RESULT(regs->rax),
regs->rax);
+		audit_syscall_exit(AUDITSC_RESULT(regs->rax),
regs->rax);
 
 	if ((test_thread_flag(TIF_SYSCALL_TRACE)
 	     || test_thread_flag(TIF_SINGLESTEP))
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 1c47c59..39fef6e 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -287,10 +287,10 @@ struct netlink_skb_parms;
 				/* Public API */
 extern int  audit_alloc(struct task_struct *task);
 extern void audit_free(struct task_struct *task);
-extern void audit_syscall_entry(struct task_struct *task, int arch,
+extern void audit_syscall_entry(int arch,
 				int major, unsigned long a0, unsigned
long a1,
 				unsigned long a2, unsigned long a3);
-extern void audit_syscall_exit(struct task_struct *task, int failed,
long return_code);
+extern void audit_syscall_exit(int failed, long return_code);
 extern void audit_getname(const char *name);
 extern void audit_putname(const char *name);
 extern void __audit_inode(const char *name, const struct inode *inode,
unsigned flags);
@@ -323,8 +323,8 @@ extern int audit_set_macxattr(const char
 #else
 #define audit_alloc(t) ({ 0; })
 #define audit_free(t) do { ; } while (0)
-#define audit_syscall_entry(t,ta,a,b,c,d,e) do { ; } while (0)
-#define audit_syscall_exit(t,f,r) do { ; } while (0)
+#define audit_syscall_entry(ta,a,b,c,d,e) do { ; } while (0)
+#define audit_syscall_exit(f,r) do { ; } while (0)
 #define audit_getname(n) do { ; } while (0)
 #define audit_putname(n) do { ; } while (0)
 #define __audit_inode(n,i,f) do { ; } while (0)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4052f0a..d2a3a88 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -329,7 +329,6 @@ static enum audit_state audit_filter_sys
 	return AUDIT_BUILD_CONTEXT;
 }
 
-/* This should be called with task_lock() held. */
 static inline struct audit_context *audit_get_context(struct
task_struct *tsk,
 						      int return_valid,
 						      int return_code)
@@ -506,7 +505,7 @@ static inline void audit_free_context(st
 		printk(KERN_ERR "audit: freed %d contexts\n", count);
 }
 
-static void audit_log_task_context(struct audit_buffer *ab, gfp_t
gfp_mask)
+static void audit_log_task_context(struct audit_buffer *ab)
 {
 	char *ctx = NULL;
 	ssize_t len = 0;
@@ -518,7 +517,7 @@ static void audit_log_task_context(struc
 		return;
 	}
 
-	ctx = kmalloc(len, gfp_mask);
+	ctx = kmalloc(len, GFP_KERNEL);
 	if (!ctx)
 		goto error_path;
 
@@ -536,47 +535,46 @@ error_path:
 	return;
 }
 
-static void audit_log_task_info(struct audit_buffer *ab, struct
task_struct *tsk, gfp_t gfp_mask)
+static void audit_log_task_info(struct audit_buffer *ab, struct
task_struct *tsk)
 {
 	char name[sizeof(tsk->comm)];
 	struct mm_struct *mm = tsk->mm;
 	struct vm_area_struct *vma;
 
+	/* tsk == current */
+
 	get_task_comm(name, tsk);
 	audit_log_format(ab, " comm=");
 	audit_log_untrustedstring(ab, name);
 
-	if (!mm)
-		return;
-
-	/*
-	 * this is brittle; all callers that pass GFP_ATOMIC will have
-	 * NULL tsk->mm and we won't get here.
-	 */
-	down_read(&mm->mmap_sem);
-	vma = mm->mmap;
-	while (vma) {
-		if ((vma->vm_flags & VM_EXECUTABLE) &&
-		    vma->vm_file) {
-			audit_log_d_path(ab, "exe=",
-					 vma->vm_file->f_dentry,
-					 vma->vm_file->f_vfsmnt);
-			break;
+	if (mm) {
+		down_read(&mm->mmap_sem);
+		vma = mm->mmap;
+		while (vma) {
+			if ((vma->vm_flags & VM_EXECUTABLE) &&
+			    vma->vm_file) {
+				audit_log_d_path(ab, "exe=",
+
vma->vm_file->f_dentry,
+
vma->vm_file->f_vfsmnt);
+				break;
+			}
+			vma = vma->vm_next;
 		}
-		vma = vma->vm_next;
+		up_read(&mm->mmap_sem);
 	}
-	up_read(&mm->mmap_sem);
-	audit_log_task_context(ab, gfp_mask);
+	audit_log_task_context(ab);
 }
 
-static void audit_log_exit(struct audit_context *context, struct
task_struct *tsk, gfp_t gfp_mask)
+static void audit_log_exit(struct audit_context *context, struct
task_struct *tsk)
 {
 	int i;
 	struct audit_buffer *ab;
 	struct audit_aux_data *aux;
 	const char *tty;
 
-	ab = audit_log_start(context, gfp_mask, AUDIT_SYSCALL);
+	/* tsk == current */
+
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_SYSCALL);
 	if (!ab)
 		return;		/* audit_panic has been called */
 	audit_log_format(ab, "arch=%x syscall=%d",
@@ -607,12 +605,12 @@ static void audit_log_exit(struct audit_
 		  context->gid,
 		  context->euid, context->suid, context->fsuid,
 		  context->egid, context->sgid, context->fsgid, tty);
-	audit_log_task_info(ab, gfp_mask);
+	audit_log_task_info(ab);
 	audit_log_end(ab);
 
 	for (aux = context->aux; aux; aux = aux->next) {
 
-		ab = audit_log_start(context, gfp_mask, aux->type);
+		ab = audit_log_start(context, GFP_KERNEL, aux->type);
 		if (!ab)
 			continue; /* audit_panic has been called */
 
@@ -649,7 +647,7 @@ static void audit_log_exit(struct audit_
 	}
 
 	if (context->pwd && context->pwdmnt) {
-		ab = audit_log_start(context, gfp_mask, AUDIT_CWD);
+		ab = audit_log_start(context, GFP_KERNEL, AUDIT_CWD);
 		if (ab) {
 			audit_log_d_path(ab, "cwd=", context->pwd,
context->pwdmnt);
 			audit_log_end(ab);
@@ -659,7 +657,7 @@ static void audit_log_exit(struct audit_
 		unsigned long ino  = context->names[i].ino;
 		unsigned long pino = context->names[i].pino;
 
-		ab = audit_log_start(context, gfp_mask, AUDIT_PATH);
+		ab = audit_log_start(context, GFP_KERNEL, AUDIT_PATH);
 		if (!ab)
 			continue; /* audit_panic has been called */
 
@@ -698,19 +696,12 @@ static void audit_log_exit(struct audit_
  * audit_free - free a per-task audit context
  * @tsk: task whose audit context block to free
  *
- * Called from copy_process and __put_task_struct.
+ * Called from copy_process and do_exit
  */
 void audit_free(struct task_struct *tsk)
 {
 	struct audit_context *context;
 
-	/*
-	 * No need to lock the task - when we execute audit_free()
-	 * then the task has no external references anymore, and
-	 * we are tearing it down. (The locking also confuses
-	 * DEBUG_LOCKDEP - this freeing may occur in softirq
-	 * contexts as well, via RCU.)
-	 */
 	context = audit_get_context(tsk, 0, 0);
 	if (likely(!context))
 		return;
@@ -719,8 +710,9 @@ void audit_free(struct task_struct *tsk)
 	 * function (e.g., exit_group), then free context block. 
 	 * We use GFP_ATOMIC here because we might be doing this 
 	 * in the context of the idle thread */
+	/* that can happen only if we are called from do_exit() */
 	if (context->in_syscall && context->auditable)
-		audit_log_exit(context, tsk, GFP_ATOMIC);
+		audit_log_exit(context, tsk);
 
 	audit_free_context(context);
 }
@@ -743,10 +735,11 @@ void audit_free(struct task_struct *tsk)
  * will only be written if another part of the kernel requests that it
  * be written).
  */
-void audit_syscall_entry(struct task_struct *tsk, int arch, int major,
+void audit_syscall_entry(int arch, int major,
 			 unsigned long a1, unsigned long a2,
 			 unsigned long a3, unsigned long a4)
 {
+	struct task_struct *tsk = current;
 	struct audit_context *context = tsk->audit_context;
 	enum audit_state     state;
 
@@ -824,22 +817,18 @@ void audit_syscall_entry(struct task_str
  * message), then write out the syscall information.  In call cases,
  * free the names stored from getname().
  */
-void audit_syscall_exit(struct task_struct *tsk, int valid, long
return_code)
+void audit_syscall_exit(int valid, long return_code)
 {
+	struct task_struct *tsk = current;
 	struct audit_context *context;
 
-	get_task_struct(tsk);
-	task_lock(tsk);
 	context = audit_get_context(tsk, valid, return_code);
-	task_unlock(tsk);
 
-	/* Not having a context here is ok, since the parent may have
-	 * called __put_task_struct. */
 	if (likely(!context))
-		goto out;
+		return;
 
 	if (context->in_syscall && context->auditable)
-		audit_log_exit(context, tsk, GFP_KERNEL);
+		audit_log_exit(context, tsk);
 
 	context->in_syscall = 0;
 	context->auditable  = 0;
@@ -854,8 +843,6 @@ void audit_syscall_exit(struct task_stru
 		audit_free_aux(context);
 		tsk->audit_context = context;
 	}
- out:
-	put_task_struct(tsk);
 }
 
 /**
diff --git a/kernel/exit.c b/kernel/exit.c
index bc0ec67..55ac4e2 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -904,6 +904,8 @@ fastcall NORET_TYPE void do_exit(long co
 	if (unlikely(tsk->compat_robust_list))
 		compat_exit_robust_list(tsk);
 #endif
+	if (unlikely(tsk->audit_context))
+		audit_free(tsk);
 	exit_mm(tsk);
 
 	exit_sem(tsk);
diff --git a/kernel/fork.c b/kernel/fork.c
index b3f7a1b..be4a935 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -116,8 +116,6 @@ void __put_task_struct_cb(struct rcu_hea
 	WARN_ON(atomic_read(&tsk->usage));
 	WARN_ON(tsk == current);
 
-	if (unlikely(tsk->audit_context))
-		audit_free(tsk);
 	security_task_free(tsk);
 	free_uid(tsk->user);
 	put_group_info(tsk->group_info);

------------------------------

--
Linux-audit mailing list
Linux-audit redhat com
https://www.redhat.com/mailman/listinfo/linux-audit

End of Linux-audit Digest, Vol 18, Issue 31
*******************************************


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]