slub check_heap_object crash

[metarox]

Hello,

Kernel 3.4 ARM

On the system I'm developing, we have PAGE_POISONING on which basically copies 0xAA bytes in free page structures. We also have android binder enabled and basically the PAX_USERCOPY code below blows up. The issue lies in that binder allocates a new page and map_vm_area's the page to an address and then later proceeds with a copy_from_user from this same address. Now with PAX_USERCOPY off, the simple fact that it writes into the address is enough to trigger a page fault which updates the current process' page table which properly maps the address to the page. But when PAX_USERCOPY is enabled, the heap check happens before the system updates the process page table and the virt_to_head_page(ptr) access below retrieves a stale poisoned cache structure (final page value is 0xaaaaaaaa because the flags attribute of the page struct also contains the same poisoned value where the tail flag is set, thus compound_page returns page->first_page which is also 0xaaaaaaaa) which leads to a crash.

Turning off PAGE_POISONING delays the crash to a later time where it triggers randomly on another stale cache that was not yet cleaned (PAX_MEMORY_SANITIZE is off).

Ideally the solution would be to force the page table to be refreshed before accessing the page from the address to make sure you get the right one.

[ 50.604906] ( 54.035156) [<c024c698>] (check_heap_object+0x9c/0x100) from [<c0256104>] (__check_object_size+0x60/0x164)
[ 50.615843] ( 54.046081) [<c0256104>] (__check_object_size+0x60/0x164) from [<c0739554>] (binder_transaction+0x920/0x11d8)
[ 50.627034] ( 54.057281) [<c0739554>] (binder_transaction+0x920/0x11d8) from [<c073a450>] (binder_thread_write+0x644/0xb50)
[ 50.638318] ( 54.068572) [<c073a450>] (binder_thread_write+0x644/0xb50) from [<c073ab8c>] (binder_ioctl+0x1e8/0x5ac)
[ 50.648997] ( 54.079223) [<c073ab8c>] (binder_ioctl+0x1e8/0x5ac) from [<c02600d4>] (vfs_ioctl+0x28/0x3c)
[ 50.658630] ( 54.088867) [<c02600d4>] (vfs_ioctl+0x28/0x3c) from [<c0260b6c>] (do_vfs_ioctl+0x4a8/0x5ac)
[ 50.668266] ( 54.098510) [<c0260b6c>] (do_vfs_ioctl+0x4a8/0x5ac) from [<c0260cb8>] (sys_ioctl+0x48/0x74)
[ 50.677865] ( 54.108093) [<c0260cb8>] (sys_ioctl+0x48/0x74) from [<c0106120>] (ret_fast_syscall+0x0/0x30)

#ifdef CONFIG_PAX_USERCOPY
const char *check_heap_object(const void *ptr, unsigned long n)
{
struct page *page;
struct kmem_cache *s;
unsigned long offset;

if (ZERO_OR_NULL_PTR(ptr))
return "<null>";

if (!virt_addr_valid(ptr))
return NULL;

page = virt_to_head_page(ptr);

if (!PageSlab(page))
return NULL;

s = page->slab;
if (!(s->flags & SLAB_USERCOPY))
return s->name;

offset = (ptr - page_address(page)) % s->size;
if (offset <= s->objsize && n <= s->objsize - offset)
return NULL;

return s->name;
}
#endif

[strcat]

Assuming this is on ARM, make sure you have "ARM: 7931/1: Correct virt_addr_valid" (efea3403d4b7c6d1dd5d5ac3234c161e8b314d66) applied. I ran into this issue using PaX with the 3.4 Android kernel and it took a while to narrow it down. It's now included in the PaX and Grsecurity 3.2 LTS patches.

[metarox]

Yes, it's ARM and kernel 3.4.

As much as I can say that it fixes the issue, isn't this fix just avoiding the check altogether which is not what this was intended in the first place?

[PaX Team]

check_heap_object wants to verify slab objects whose addresses will pass the (properly implemented) virt_addr_valid check (and the rest too of course), so it's very much intended.