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

[Hugh Dickins]

On Tue, Apr 5, 2011 at 8:37 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Apr 5, 2011 at 5:21 AM, Robert ÅwiÄcki <robert@xxxxxxxxxxx> 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 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/