On Wed, Apr 6, 2011 at 8:43 AM, Hugh Dickins <hughd@xxxxxxxxxx> wrote:
>
> I was about to send you my own UNTESTED patch: let me append it anyway,
> I think it is more correct than yours (it's the offset of vm_end we need
> to worry about, and there's the funny old_len,new_len stuff).
Umm. That's what my patch did too. The
pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
is the "offset of the pgoff" from the original mapping, then we do
pgoff += vma->vm_pgoff;
to get the pgoff of the new mapping, and then we do
if (pgoff + (new_len >> PAGE_SHIFT) < pgoff)
to check that the new mapping is ok.
I think yours is equivalent, just a different (and odd - that
linear_page_index() thing will do lots of unnecessary shifts and
hugepage crap) way of writing it.
> See what you think - sorry, I'm going out now.
I think _yours_ is conceptually buggy, because I think that test for
"vma->vm_file" is wrong.
Yes, new anonymous mappings set vm_pgoff to the virtual address, but
that's not true for mremap() moving them around, afaik.
Admittedly it's really hard to get to the overflow case, because the
address is shifted down, so even if you start out with an anonymous
mmap at a high address (to get a big vm_off), and then move it down
and expand it (to get a big size), I doubt you can possibly overflow.
But I still don't think that the test for vm_file is semantically
sensible, even if it might not _matter_.
But whatever. I suspect both our patches are practically doing the
same thing, and it would be interesting to hear if it actually fixes
the issue. Maybe there is some other way to mess up vm_pgoff that I
can't think of right now.
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href