Re: [PATCH] audit: destroy filename correctly PING.
by Dmitry Monakhov
On Mon, 1 Apr 2013 11:00:00 +0400, Dmitry Monakhov <dmonakhov(a)openvz.org> wrote:
Ping. Patch (https://lkml.org/lkml/2013/4/1/65) was not a 1'st April's joke.
Add CC:linux-audit@redhat.com
> filename should be destroyed via final_putname() instead of __putname()
> Otherwise this result in following BUGON() in case of long names:
> kernel BUG at mm/slab.c:3006!
> Call Trace:
> kmem_cache_free+0x1c1/0x850
> audit_putname+0x88/0x90
> putname+0x73/0x80
> sys_symlinkat+0x120/0x150
> sys_symlink+0x16/0x20
> system_call_fastpath+0x16/0x1b
>
> Signed-off-by: Dmitry Monakhov <dmonakhov(a)openvz.org>
> ---
> kernel/auditsc.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index a371f85..bfe7ca6 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1010,7 +1010,7 @@ static inline void audit_free_names(struct audit_context *context)
> list_for_each_entry_safe(n, next, &context->names_list, list) {
> list_del(&n->list);
> if (n->name && n->name_put)
> - __putname(n->name);
> + final_putname(n->name);
> if (n->should_free)
> kfree(n);
> }
> @@ -2036,7 +2036,7 @@ void audit_putname(struct filename *name)
> BUG_ON(!context);
> if (!context->in_syscall) {
> #if AUDIT_DEBUG == 2
> - printk(KERN_ERR "%s:%d(:%d): __putname(%p)\n",
> + printk(KERN_ERR "%s:%d(:%d): final_putname(%p)\n",
> __FILE__, __LINE__, context->serial, name);
> if (context->name_count) {
> struct audit_names *n;
> @@ -2047,7 +2047,7 @@ void audit_putname(struct filename *name)
> n->name, n->name->name ?: "(null)");
> }
> #endif
> - __putname(name);
> + final_putname(name);
> }
> #if AUDIT_DEBUG
> else {
> --
> 1.7.1
>
11 years, 6 months
[PATCH] vfs: fix audit_inode call in O_CREAT case of do_last
by Jeff Layton
Jiri reported a regression in auditing of open(..., O_CREAT) syscalls.
In older kernels, creating a file with open(..., O_CREAT) created
audit_name records that looked like this:
type=PATH msg=audit(1360255720.628:64): item=1 name="/abc/foo" inode=138810 dev=fd:00 mode=0100640 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:default_t:s0
type=PATH msg=audit(1360255720.628:64): item=0 name="/abc/" inode=138635 dev=fd:00 mode=040750 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:default_t:s0
...in recent kernels though, they look like this:
type=PATH msg=audit(1360255402.886:12574): item=2 name=(null) inode=264599 dev=fd:00 mode=0100640 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:default_t:s0
type=PATH msg=audit(1360255402.886:12574): item=1 name=(null) inode=264598 dev=fd:00 mode=040750 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:default_t:s0
type=PATH msg=audit(1360255402.886:12574): item=0 name="/abc/foo" inode=264598 dev=fd:00 mode=040750 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:default_t:s0
Richard bisected to determine that the problems started with commit
bfcec708, but the log messages have changed with some later
audit-related patches.
The problem is that this audit_inode call is passing in the parent of
the dentry being opened, but audit_inode is being called with the parent
flag false. This causes later audit_inode and audit_inode_child calls to
match the wrong entry in the audit_names list.
This patch simply sets the flag to properly indicate that this inode
represents the parent. With this, the audit_names entries are back to
looking like they did before.
Cc: <stable(a)vger.kernel.org> # v3.7+
Cc: Richard Guy Briggs <rbriggs(a)redhat.com>
Reported-by: Jiri Jaburek <jjaburek(a)redhat.com>
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
fs/namei.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 57ae9c8..85e40d1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2740,7 +2740,7 @@ static int do_last(struct nameidata *nd, struct path *path,
if (error)
return error;
- audit_inode(name, dir, 0);
+ audit_inode(name, dir, LOOKUP_PARENT);
error = -EISDIR;
/* trailing slashes? */
if (nd->last.name[nd->last.len])
--
1.7.1
11 years, 6 months
[PATCH 1/2] audit: remove duplicate export of audit_enabled
by Gao feng
audit_enabled has already been exported in
include/linux/audit.h. and kernel/audit.h
includes include/linux/audit.h, no need to
export aduit_enabled again in kernel/audit.h
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
kernel/audit.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/audit.h b/kernel/audit.h
index d51cba8..d06ffc1 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -60,7 +60,6 @@ struct audit_entry {
};
#ifdef CONFIG_AUDIT
-extern int audit_enabled;
extern int audit_ever_enabled;
#endif
--
1.8.1.4
11 years, 6 months
[PATCH] audit: fix build break when AUDIT_DEBUG == 2
by Jeff Layton
Looks like this one has been around since 5195d8e21:
kernel/auditsc.c: In function ‘audit_free_names’:
kernel/auditsc.c:998: error: ‘i’ undeclared (first use in this function)
...and this warning:
kernel/auditsc.c: In function ‘audit_putname’:
kernel/auditsc.c:2045: warning: ‘i’ may be used uninitialized in this function
Cc: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
kernel/auditsc.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index a371f85..4083b69 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -987,6 +987,8 @@ static inline void audit_free_names(struct audit_context *context)
#if AUDIT_DEBUG == 2
if (context->put_count + context->ino_count != context->name_count) {
+ int i = 0;
+
printk(KERN_ERR "%s:%d(:%d): major=%d in_syscall=%d"
" name_count=%d put_count=%d"
" ino_count=%d [NOT freeing]\n",
@@ -995,7 +997,7 @@ static inline void audit_free_names(struct audit_context *context)
context->name_count, context->put_count,
context->ino_count);
list_for_each_entry(n, &context->names_list, list) {
- printk(KERN_ERR "names[%d] = %p = %s\n", i,
+ printk(KERN_ERR "names[%d] = %p = %s\n", i++,
n->name, n->name->name ?: "(null)");
}
dump_stack();
@@ -2040,10 +2042,10 @@ void audit_putname(struct filename *name)
__FILE__, __LINE__, context->serial, name);
if (context->name_count) {
struct audit_names *n;
- int i;
+ int i = 0;
list_for_each_entry(n, &context->names_list, list)
- printk(KERN_ERR "name[%d] = %p = %s\n", i,
+ printk(KERN_ERR "name[%d] = %p = %s\n", i++,
n->name, n->name->name ?: "(null)");
}
#endif
--
1.7.1
11 years, 6 months
[PATCH RESEND] audit: don't check if kauditd is valid everytime
by Gao feng
We only need to check if kauditd is valid after we start
it, if kauditd is invalid, we will set kauditd_task to NULL.
So next time, we will start kauditd again.
It means if kauditd_task is not NULL,it must be valid.
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
kernel/audit.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index d596e53..c3a6ead 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -660,12 +660,13 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
/* As soon as there's any sign of userspace auditd,
* start kauditd to talk to it */
- if (!kauditd_task)
+ if (!kauditd_task) {
kauditd_task = kthread_run(kauditd_thread, NULL, "kauditd");
- if (IS_ERR(kauditd_task)) {
- err = PTR_ERR(kauditd_task);
- kauditd_task = NULL;
- return err;
+ if (IS_ERR(kauditd_task)) {
+ err = PTR_ERR(kauditd_task);
+ kauditd_task = NULL;
+ return err;
+ }
}
loginuid = audit_get_loginuid(current);
--
1.7.11.7
11 years, 6 months
How to offer a patch
by Burn Alting
All,
I've made some mods to auditctl to allow it to read a directory of 'rule
files'. The idea is that within an enterprise, one would distribute a
standard /etc/audit/audit.rules which can be updated from the corporate
repository. Should a system require localized audit rules, then a
directory containing files of rules can be maintained locally. The
reasoning for a directory as opposed to just an additional file is to
offer granularity of 'rule sets'.
I would like to know the convention for patching to this list. Should I
git clone the svn repository then supply a git diff? Can I just provide
an old-fashioned diff -rupN or C_ALL=C TZ=UTC0 diff -Naur?
Regards
Burn Alting
11 years, 7 months
