Messages in this thread | | | Date | Sat, 19 Sep 2009 10:06:09 +0200 | From | Ingo Molnar <> | Subject | Re: perf_copy_attr pointer arithmetic weirdness |
| |
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, 2009-09-18 at 21:26 +0200, Ian Schram wrote: > > There is some -to me at least- weird code in per_copy_attr. Which supposedly > > checks that all bytes trailing a struct are zero. > > > > It doesn't seem to get pointer arithmetic right. Since it increments > > an iterating pointer by sizeof(unsigned long) rather than 1. > > > > I believe this has an impact on the exploitability of the recent buffer overflow > > in the perf_copy_attr function. I'm pretty sure I'm not the only one who noticed > > this, but i couldn't find it being mentioned. For some reason people prefer > > mmaping something at zero these days? > > > > I have appended a patch locating the issue. The PTR_ALIGN stuff right above it > > doesn't seem to take any boundary conditions into account which is probably not > > a good thing either. > > sizeof(struct perf_counter_attr) should always be a multiple of u64, and > we can indeed read beyond the tail boundary, but that should be ok, > worst that can happen is that we fail the read.. > > Ugh on the ptr arith, one wonders how many stupid bugs one can make in > such a piece of code... :/ > > > signed-of-by Ian Schram <ischram@telenet.be> > > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Ian, you meant Signed-off-by, not signed-of-by, right?
Ingo
| |