perf: bug, kernel ignores the buffer size on large read

perf: bug, kernel ignores the buffer size on large read

[Vince Weaver]

OK, this time I found the actual bug.

event->read_size is declared as a u16 in include/linux/perf_event.h

but it is very easy to get event->read_size larger than 64k
(in my case, create 10000 events in a group).

Because we wrap around the u16, the 
	 if (count < event->read_size) return -ENOSPC;
in perf_read_hw() doesn't trigger reliably and so if you do a read
on a large group event the kernel will quite happily copy_to_user()
beyond the bounds of the value set in the read syscall.  

In my case it completely smashed the stack and caused the program to 
segfault.

I'm not sure what the solution is here.  Change read_size to be larger?
Ban events whose read size would be larger than 64k?  Although that gets 
tricky because the related header_size is also only a u16.

Vince

Re: perf: bug, kernel ignores the buffer size on large read

[Peter Zijlstra]

On Wed, Sep 02, 2015 at 12:34:41PM -0400, Vince Weaver wrote:
> 
> OK, this time I found the actual bug.
> 
> event->read_size is declared as a u16 in include/linux/perf_event.h
> 
> but it is very easy to get event->read_size larger than 64k
> (in my case, create 10000 events in a group).
> 
> Because we wrap around the u16, the 
> 	 if (count < event->read_size) return -ENOSPC;
> in perf_read_hw() doesn't trigger reliably and so if you do a read
> on a large group event the kernel will quite happily copy_to_user()
> beyond the bounds of the value set in the read syscall.  
> 
> In my case it completely smashed the stack and caused the program to 
> segfault.
> 
> I'm not sure what the solution is here.  Change read_size to be larger?
> Ban events whose read size would be larger than 64k?  Although that gets 
> tricky because the related header_size is also only a u16.

I think we should try and just ban events with a read_size > 64k; that's
one _large_ group -- there's other issues with that as well I imagine.

If we really want to allow something larger, we _could_ fudge something
for cases where we do not have PERF_SAMPLE_READ set, but that's not
particularly nice either.

Re: perf: bug, kernel ignores the buffer size on large read

[Vince Weaver]

On Thu, 3 Sep 2015, Peter Zijlstra wrote:

> I think we should try and just ban events with a read_size > 64k; that's
> one _large_ group -- there's other issues with that as well I imagine.

yes, I don't really have a use case for this, I just noticed the problem 
because Debian apparently bumped the default file descriptor limit from
1k to 64k and suddenly some of my perf_event_test suite started failing
in interesting ways.

The limit for read() would come down to about 4k events per group assuming 
you also have ID enabled.

The complication is that header_size is calculated similarly and lots of 
other things get added in, and a lot of it is done with u16 math.  So I 
have no idea if there's a possibility for creating sampled events that can 
corrupt the mmap buffer.

Vince