2008-08-01 11:18:19

by Zhang Xiliang

[permalink] [raw]
Subject: [PATCH] Fix the kernel panic of audit_filter_task when key field is set

When calling audit_filter_task(), it calls audit_filter_rules() with audit_context is NULL.
If the key field is set, the result in audit_filter_rules() will be set to 1 and
ctx->filterkey will be set to key.
But the ctx is NULL in this condition, so kernel will panic.

Signed-off-by: Zhang Xiliang <[email protected]>
---
kernel/auditsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4699950..012c94e 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -610,7 +610,7 @@ static int audit_filter_rules(struct task_struct *tsk,
if (!result)
return 0;
}
- if (rule->filterkey)
+ if (rule->filterkey && ctx)
ctx->filterkey = kstrdup(rule->filterkey, GFP_ATOMIC);
switch (rule->action) {
case AUDIT_NEVER: *state = AUDIT_DISABLED; break;


2008-08-02 02:21:45

by Zhang Xiliang

[permalink] [raw]
Subject: Re: [PATCH] Fix the kernel panic of audit_filter_task when key field is set

[PATCH] Fix the kernel panic of audit_filter_task when AUDIT_PERM or AUDIT_FILETYPE field is set

When calling audit_filter_task(), it calls audit_filter_rules() with audit_context is NULL.
If the AUDIT_PERM or AUDIT_FILETYPE field is set, audit_match_perm() or audit_match_filetype() will use ctx->xx.
But the ctx is NULL in this condition, so kernel will panic.

Signed-off-by: Zhang Xiliang <[email protected]>
---
kernel/auditsc.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 012c94e..29b6964 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -243,6 +243,8 @@ static inline int open_arg(int flags, int mask)

static int audit_match_perm(struct audit_context *ctx, int mask)
{
+ if(!ctx)
+ return 0;
unsigned n = ctx->major;
switch (audit_classify_syscall(ctx->arch, n)) {
case 0: /* native */
@@ -284,6 +286,8 @@ static int audit_match_filetype(struct audit_context *ctx, int which)
{
unsigned index = which & ~S_IFMT;
mode_t mode = which & S_IFMT;
+ if(!ctx)
+ return 0;
if (index >= ctx->name_count)
return 0;
if (ctx->names[index].ino == -1)


zhangxiliang said the following on 2008-08-01 19:15:
> When calling audit_filter_task(), it calls audit_filter_rules() with audit_context is NULL.
> If the key field is set, the result in audit_filter_rules() will be set to 1 and
> ctx->filterkey will be set to key.
> But the ctx is NULL in this condition, so kernel will panic.
>
> Signed-off-by: Zhang Xiliang <[email protected]>
> ---
> kernel/auditsc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4699950..012c94e 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -610,7 +610,7 @@ static int audit_filter_rules(struct task_struct *tsk,
> if (!result)
> return 0;
> }
> - if (rule->filterkey)
> + if (rule->filterkey && ctx)
> ctx->filterkey = kstrdup(rule->filterkey, GFP_ATOMIC);
> switch (rule->action) {
> case AUDIT_NEVER: *state = AUDIT_DISABLED; break;
>
> --
> Linux-audit mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/linux-audit
>
>
>

2008-08-02 02:51:47

by Yu Zhiguo

[permalink] [raw]
Subject: Re: [PATCH] Fix the kernel panic of audit_filter_task when key field is set


zhangxiliang wrote:

> static int audit_match_perm(struct audit_context *ctx, int mask)
> {
> + if(!ctx)
> + return 0;
> unsigned n = ctx->major;

Please check this patch with scripts/checkpatch.pl and then resend it.


> switch (audit_classify_syscall(ctx->arch, n)) {
> case 0: /* native */
> @@ -284,6 +286,8 @@ static int audit_match_filetype(struct audit_context *ctx, int which)
> {
> unsigned index = which & ~S_IFMT;
> mode_t mode = which & S_IFMT;
> + if(!ctx)
> + return 0;
> if (index >= ctx->name_count)
> return 0;
> if (ctx->names[index].ino == -1)
>
>

2008-08-02 02:58:56

by Zhang Xiliang

[permalink] [raw]
Subject: Re: [PATCH] Fix the kernel panic of audit_filter_task when key field is set

Sorry, I miss a blank between if and "(".
And I add "unlikely" to check "ctx" in audit_match_perm() and audit_match_filetype().
This is a new patch for it.

Signed-off-by: Zhang Xiliang <[email protected]>
---
kernel/auditsc.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4699950..57a001a 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -243,6 +243,9 @@ static inline int open_arg(int flags, int mask)

static int audit_match_perm(struct audit_context *ctx, int mask)
{
+ if (unlikely(!ctx))
+ return 0;
+
unsigned n = ctx->major;
switch (audit_classify_syscall(ctx->arch, n)) {
case 0: /* native */
@@ -284,6 +287,10 @@ static int audit_match_filetype(struct audit_context *ctx, int which)
{
unsigned index = which & ~S_IFMT;
mode_t mode = which & S_IFMT;
+
+ if (unlikely(!ctx))
+ return 0;
+
if (index >= ctx->name_count)
return 0;
if (ctx->names[index].ino == -1)

Yu Zhiguo said the following on 2008-08-02 10:51:
>
> zhangxiliang wrote:
>
>> static int audit_match_perm(struct audit_context *ctx, int mask)
>> {
>> + if(!ctx)
>> + return 0;
>> unsigned n = ctx->major;
>
> Please check this patch with scripts/checkpatch.pl and then resend it.
>
>
>> switch (audit_classify_syscall(ctx->arch, n)) {
>> case 0: /* native */
>> @@ -284,6 +286,8 @@ static int audit_match_filetype(struct
>> audit_context *ctx, int which)
>> {
>> unsigned index = which & ~S_IFMT;
>> mode_t mode = which & S_IFMT;
>> + if(!ctx)
>> + return 0;
>> if (index >= ctx->name_count)
>> return 0;
>> if (ctx->names[index].ino == -1)
>>
>>
>
>
>
>

2008-08-04 10:20:42

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] Fix the kernel panic of audit_filter_task when key field is set

On Sat, Aug 02, 2008 at 10:56:37AM +0800, zhangxiliang wrote:
> Sorry, I miss a blank between if and "(".
> And I add "unlikely" to check "ctx" in audit_match_perm() and audit_match_filetype().
> This is a new patch for it.

Thanks. Applied and pushed.

2008-08-20 11:18:28

by Benny Halevy

[permalink] [raw]
Subject: [PATCH] fix warning in audit_match_perm


kernel/auditsc.c: In function ‘audit_match_perm’:
kernel/auditsc.c:249: warning: ISO C90 forbids mixed declarations and code

This was introduced in commit
1a61c88defcd611bd148d6c960b498e1b8bbbe00

Signed-off-by: Benny Halevy <[email protected]>
---
kernel/auditsc.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 972f8e6..e72b161 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -243,10 +243,12 @@ static inline int open_arg(int flags, int mask)

static int audit_match_perm(struct audit_context *ctx, int mask)
{
+ unsigned n;
+
if (unlikely(!ctx))
return 0;

- unsigned n = ctx->major;
+ n = ctx->major;
switch (audit_classify_syscall(ctx->arch, n)) {
case 0: /* native */
if ((mask & AUDIT_PERM_WRITE) &&
--
1.6.0