Re: [PATCH] mm: fix possible cause of a page_mapped BUG

[Hugh Dickins <hughd@google.com>]

On Tue, Apr 5, 2011 at 8:37 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Apr 5, 2011 at 5:21 AM, Robert Święcki <robert@swiecki.net> wrote:
>>
>> Here it is, I'll leave it in this state (kdb) in case you need some
>> remote debugging
>>
>> <4>[ 1523.877666] WARNING: at mm/prio_tree.c:95 vma_prio_tree_add+0x43/0x110()
>> <4>[ 1523.978650] vm_area_struct at ffff880120bda508:
>> <4>[ 1523.983199]  ffff88011eb5aa00 00000000f72f3000 00000000f73f0000 ffff88011b8eaa10
>> <4>[ 1523.990674]  ffff88011b8ea228 0000000000000027 00000000000101ff ffff88011b8ea6b1
>> <4>[ 1523.998151]  ffff88011e390820 ffff88011b8ea260 ffff880120796780 ffff880120bdad40
>> <4>[ 1524.005624]            (null)           (null) ffff88011ed5b910 ffff88011ed5b1f0
>> <4>[ 1524.013103]  ffff88011f72b168 ffffffff82427480 ffffffffffffff03 ffff8800793ff0c0
>> <4>[ 1524.020581]            (null)           (null)           (null)
>
> vma->vm_start/end is 0xf72f3000-0xf73f0000
>
>> <4>[ 1524.026556] vm_area_struct at ffff880120bdacf0:
>> <4>[ 1524.031110]  ffff88011eb5a300 00000000f72f3000 00000000f7400000 ffff88011f6c6f18
>> <4>[ 1524.038584]  ffff88011b5c9da8 0000000000000027 00000000000101ff ffff8801206f0c71
>> <4>[ 1524.046062]  ffff88011f6c6f50 ffff88011b5c9de0 ffff880120bdad40 ffff880120bdad40
>> <4>[ 1524.053536]  ffff880120bda558           (null) ffff88011f758ee0 ffff88011f7583a0
>> <4>[ 1524.061016]  ffff88011f556690 ffffffff82427480 ffffffffffffff03 ffff8800793ff0c0
>> <4>[ 1524.068491]            (null)           (null)           (null)
>
> vma->vm_start/end is 0xf72f3000-0xf7400000.
>
> If I read those right, then the vm_pgoff (RADIX_INDEX for the
> prio-tree) is ffffffffffffff03 for both cases. That doesn't look good.
> How do we get a negative pg_off for a file mapping?

Yes, I think that's probably at the root of it.  Robert is using a
fuzzer, and it's a 32-bit executable running on a 64-bit kernel: I
suspect there's somewhere on our compat path where we've not validated
incoming mmap offset properly.

Hmm, but I don't see anything wrong there.

>
> Also, since they have a different size, they should have a different
> HEAP_INDEX. That's why we BUG_ON() - with a different HEAP_INDEX,
> shouldn't that mean that the prio_tree_insert() logic should create a
> new node for it?

Yes.

>
> I dunno. But that odd negative pg_off thing makes me think there is
> some overflow issue (ie HEAP_INDEX being pg_off + size ends up
> fluctuating between really big and really small). So I'd suspect THAT
> as the main reason.

Yes, one of the vmas is such that the end offset (pgoff of next page
after) would be 0, and for the other it would be 16.  There's sure to
be places, inside the prio_tree code and outside it, where we rely
upon pgoff not wrapping around - wrap should be prevented by original
validation of arguments.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  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=mailto:"dont@kvack.org"> email@kvack.org </a>