2009-09-15 06:45:26

by Xiao Guangrong

[permalink] [raw]
Subject: [PATCH] perf_counter: fix buffer overflow in perf_copy_attr()

If we pass a big size data over perf_counter_open syscall, the kernel
will copy this data to a small buffer, It will cause kernel crash.

This bug make kernel unsafe and no-root user can trigger it.

Signed-off-by: Xiao Guangrong <[email protected]>
---
kernel/perf_counter.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 667ab25..75c46c0 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -4216,6 +4216,7 @@ static int perf_copy_attr(struct perf_counter_attr __user *uattr,
if (val)
goto err_size;
}
+ size = sizeof(*attr);
}

ret = copy_from_user(attr, uattr, size);
--
1.6.1.2


2009-09-15 06:56:06

by Paul Mackerras

[permalink] [raw]
Subject: Re: [PATCH] perf_counter: fix buffer overflow in perf_copy_attr()

Xiao Guangrong writes:

> If we pass a big size data over perf_counter_open syscall, the kernel
> will copy this data to a small buffer, It will cause kernel crash.
>
> This bug make kernel unsafe and no-root user can trigger it.
>
> Signed-off-by: Xiao Guangrong <[email protected]>
> ---
> kernel/perf_counter.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
> index 667ab25..75c46c0 100644
> --- a/kernel/perf_counter.c
> +++ b/kernel/perf_counter.c
> @@ -4216,6 +4216,7 @@ static int perf_copy_attr(struct perf_counter_attr __user *uattr,
> if (val)
> goto err_size;
> }
> + size = sizeof(*attr);

Looks right to me.

Acked-by: Paul Mackerras <[email protected]>

2009-09-15 07:40:24

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] perf_counter: fix buffer overflow in perf_copy_attr()

On Tue, 2009-09-15 at 14:44 +0800, Xiao Guangrong wrote:
> If we pass a big size data over perf_counter_open syscall, the kernel
> will copy this data to a small buffer, It will cause kernel crash.
>
> This bug make kernel unsafe and no-root user can trigger it.

Ah, indeed. Thanks!

Acked-by: Peter Zijlstra <[email protected]>

> Signed-off-by: Xiao Guangrong <[email protected]>
> ---
> kernel/perf_counter.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
> index 667ab25..75c46c0 100644
> --- a/kernel/perf_counter.c
> +++ b/kernel/perf_counter.c
> @@ -4216,6 +4216,7 @@ static int perf_copy_attr(struct perf_counter_attr __user *uattr,
> if (val)
> goto err_size;
> }
> + size = sizeof(*attr);
> }
>
> ret = copy_from_user(attr, uattr, size);

2009-09-15 09:22:12

by Xiao Guangrong

[permalink] [raw]
Subject: [tip:perfcounters/urgent] perf_counter: Fix buffer overflow in perf_copy_attr()

Commit-ID: b3e62e35058fc744ac794611f4e79bcd1c5a4b83
Gitweb: http://git.kernel.org/tip/b3e62e35058fc744ac794611f4e79bcd1c5a4b83
Author: Xiao Guangrong <[email protected]>
AuthorDate: Tue, 15 Sep 2009 14:44:36 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 15 Sep 2009 09:53:31 +0200

perf_counter: Fix buffer overflow in perf_copy_attr()

If we pass a big size data over perf_counter_open() syscall,
the kernel will copy this data to a small buffer, it will
cause kernel crash.

This bug makes the kernel unsafe and non-root local user can
trigger it.

Signed-off-by: Xiao Guangrong <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Acked-by: Paul Mackerras <[email protected]>
Cc: <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
kernel/perf_counter.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index d7cbc57..a67a1dc 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -4171,6 +4171,7 @@ static int perf_copy_attr(struct perf_counter_attr __user *uattr,
if (val)
goto err_size;
}
+ size = sizeof(*attr);
}

ret = copy_from_user(attr, uattr, size);