Re: [perf] yet another 32/64-bit range check failure

[Vince Weaver]

On Wed, 23 Apr 2014, Peter Zijlstra wrote:

> On Tue, Apr 22, 2014 at 11:40:07PM -0400, Vince Weaver wrote:
> > 
> > More fun found by the perf_fuzzer...
> > 
> > In kernel/events/core.c 
> > SYSCALL_DEFINE5(perf_event_open,
> > 
> > We check if flags is valid like this:
> > 
> >         /* for future expandability... */
> >         if (flags & ~PERF_FLAG_ALL)
> >                 return -EINVAL;
> > 
> > but flags is a 64-bit value but ~PERF_FLAG_ALL is 32-bit.
> > 
> > This means values like 0x800000000000ULL are treated as valid even though 
> > they aren't.
> > 
> > This is allowing events to be allocated memory but not being freed somehow
> > before returning EINVAL (a memory leak).
> > At least it looks like this is happening in the huge traces I have trying 
> > to track down the perf_fuzzer memory corruption bug.
> > 
> > I'd send a patch to fix the above, but it's late and I can't figure out 
> > where exactly to stick ULL to get PERF_FLAG_ALL to be upgraded to 64-bit.
> > 
> > Vince
> 
> Something like so should do I suppose.
> 
> ---
> Subject: perf: Fix perf_event_open(.flags) test
> 
> Vince noticed that we test the (unsigned long) flags field against an
> (unsigned int) constant. This would allow setting the high bits on 64bit
> platforms and not get an error.
> 
> There is nothing that uses the high bits, so it should be entirely
> harmless, but we don't want userspace to accidentally set them anyway,
> so fix the constants.

I suppose I should make a patch for attr->sample_type and 
attr->read_format which after a quick audit seem to exhibit the same 
problem?

Vince
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/