2013-04-13 19:50:06

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH] perf: treat attr.config as u64 in perf_swevent_init()

Trinity discovered that we fail to check all 64 bits of attr.config
passed by user space, resulting to out-of-bounds access of the
perf_swevent_enabled array in sw_perf_event_destroy().

Introduced in commit b0a873ebb ("perf: Register PMU implementations").

Signed-off-by: Tommi Rantala <[email protected]>
Cc: Peter Zijlstra <[email protected]>
---
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 59412d0..fff6420 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5330,7 +5330,7 @@ static void sw_perf_event_destroy(struct perf_event *event)

static int perf_swevent_init(struct perf_event *event)
{
- int event_id = event->attr.config;
+ u64 event_id = event->attr.config;

if (event->attr.type != PERF_TYPE_SOFTWARE)
return -ENOENT;
--
1.8.1.4


Subject: [tip:perf/urgent] perf: Treat attr.config as u64 in perf_swevent_init()

Commit-ID: 8176cced706b5e5d15887584150764894e94e02f
Gitweb: http://git.kernel.org/tip/8176cced706b5e5d15887584150764894e94e02f
Author: Tommi Rantala <[email protected]>
AuthorDate: Sat, 13 Apr 2013 22:49:14 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 15 Apr 2013 11:42:12 +0200

perf: Treat attr.config as u64 in perf_swevent_init()

Trinity discovered that we fail to check all 64 bits of
attr.config passed by user space, resulting to out-of-bounds
access of the perf_swevent_enabled array in
sw_perf_event_destroy().

Introduced in commit b0a873ebb ("perf: Register PMU
implementations").

Signed-off-by: Tommi Rantala <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7e0962e..4d3124b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5331,7 +5331,7 @@ static void sw_perf_event_destroy(struct perf_event *event)

static int perf_swevent_init(struct perf_event *event)
{
- int event_id = event->attr.config;
+ u64 event_id = event->attr.config;

if (event->attr.type != PERF_TYPE_SOFTWARE)
return -ENOENT;

2013-04-18 11:17:16

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [tip:perf/urgent] perf: Treat attr.config as u64 in perf_swevent_init()

On Mon, 2013-04-15 at 03:42 -0700, tip-bot for Tommi Rantala wrote:
> Commit-ID: 8176cced706b5e5d15887584150764894e94e02f
> Gitweb: http://git.kernel.org/tip/8176cced706b5e5d15887584150764894e94e02f
> Author: Tommi Rantala <[email protected]>
> AuthorDate: Sat, 13 Apr 2013 22:49:14 +0300
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Mon, 15 Apr 2013 11:42:12 +0200
>
> perf: Treat attr.config as u64 in perf_swevent_init()
>
> Trinity discovered that we fail to check all 64 bits of
> attr.config passed by user space, resulting to out-of-bounds
> access of the perf_swevent_enabled array in
> sw_perf_event_destroy().

Gah, I so missed we could hide bits in the top word and then use them
in _destroy().

The alternative is of course to also truncate to int in _destroy(), but
yes keeping the natural size seems the best alternative.

2013-04-22 12:40:26

by Vince Weaver

[permalink] [raw]
Subject: Re: [tip:perf/urgent] perf: Treat attr.config as u64 in perf_swevent_init()

On Thu, 18 Apr 2013, Peter Zijlstra wrote:

> On Mon, 2013-04-15 at 03:42 -0700, tip-bot for Tommi Rantala wrote:
> > Commit-ID: 8176cced706b5e5d15887584150764894e94e02f
> > Gitweb: http://git.kernel.org/tip/8176cced706b5e5d15887584150764894e94e02f
> > Author: Tommi Rantala <[email protected]>
> > AuthorDate: Sat, 13 Apr 2013 22:49:14 +0300
> > Committer: Ingo Molnar <[email protected]>
> > CommitDate: Mon, 15 Apr 2013 11:42:12 +0200
> >
> > perf: Treat attr.config as u64 in perf_swevent_init()
> >
> > Trinity discovered that we fail to check all 64 bits of
> > attr.config passed by user space, resulting to out-of-bounds
> > access of the perf_swevent_enabled array in
> > sw_perf_event_destroy().
>
> Gah, I so missed we could hide bits in the top word and then use them
> in _destroy().
>
> The alternative is of course to also truncate to int in _destroy(), but
> yes keeping the natural size seems the best alternative.

has this been marked for stable now that it's in 3.9-rc8? It's trivial to
oops/lock the kernel with a few line program and the problem has been
around a while.

Vince