linux-kernel - [PATCH] drm/ttm: fix out-of-bounds read in ttm_put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Mar 2019 17:32:38 +0100
From:   Jann Horn <jannh@...gle.com>
To:     Christian Koenig <christian.koenig@....com>,
        Huang Rui <ray.huang@....com>,
        Junwei Zhang <Jerry.Zhang@....com>, jannh@...gle.com
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: [PATCH] drm/ttm: fix out-of-bounds read in ttm_put_pages()

When ttm_put_pages() tries to figure out whether it's dealing with
transparent hugepages, it just reads past the bounds of the pages array
without a check; instead, when the end of the array is reached, treat that
as "we don't have enough pages left for a hugepage, so this isn't a
hugepage".


This was discovered by booting a libvirt VM with QXL graphics, which causes
a KASAN splat. When I add the following line at the start of
ttm_page_pool_get_pages():

    pr_warn("ttm_put_pages(0x%lx, 0x%x)\n", (unsigned long)pages, npages);

I get this dmesg output:

[TTM] ttm_put_pages(0xffff8881e68c7ac0, 0x1)
[...]
==================================================================
[...]
BUG: KASAN: slab-out-of-bounds in ttm_put_pages+0x5a0/0x680
Read of size 8 at addr ffff8881e68c7ac8 by task kworker/3:2/189
[...]
CPU: 3 PID: 189 Comm: kworker/3:2 Not tainted 5.1.0-rc2+ #337
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[...]
Call Trace:
[...]
 kasan_report+0x14e/0x192
[...]
 ttm_put_pages+0x5a0/0x680
[...]
 ttm_pool_unpopulate_helper+0xcb/0xf0
 ttm_tt_destroy.part.11+0x80/0x90
 ttm_bo_cleanup_memtype_use+0x5f/0xe0
 ttm_bo_cleanup_refs+0x262/0x2a0
 ttm_bo_delayed_delete+0x2cb/0x2f0
[...]
 ttm_bo_delayed_workqueue+0x17/0x50
 process_one_work+0x452/0x7d0
 worker_thread+0x69/0x690
 ? process_one_work+0x7d0/0x7d0
 kthread+0x1ac/0x1d0
 ? kthread_park+0xb0/0xb0
 ret_from_fork+0x1f/0x30

Allocated by task 189:
 __kasan_kmalloc.constprop.9+0xa0/0xd0
 __kmalloc_node+0x132/0x320
 ttm_tt_init+0xc2/0x120
 qxl_ttm_tt_create+0x77/0xa0
 ttm_tt_create+0x90/0xf0
 ttm_bo_handle_move_mem+0xc14/0xca0
 ttm_bo_evict+0x240/0x540
 ttm_mem_evict_first+0x240/0x2f0
 ttm_bo_mem_space+0x492/0x660
 ttm_bo_validate+0x18b/0x220
 ttm_bo_init_reserved+0x603/0x6a0
 ttm_bo_init+0xc7/0x1b0
 qxl_bo_create+0x185/0x200
 qxl_alloc_bo_reserved+0x7e/0x100
 qxl_image_alloc_objects+0xdf/0x1a0
 qxl_draw_dirty_fb+0x2ec/0x9bc
 qxl_framebuffer_surface_dirty+0x15c/0x1f0
 drm_fb_helper_dirty_work+0x25e/0x2a0
 process_one_work+0x452/0x7d0
 worker_thread+0x69/0x690
 kthread+0x1ac/0x1d0
 ret_from_fork+0x1f/0x30
[...]
The buggy address belongs to the object at ffff8881e68c7ac0
                which belongs to the cache kmalloc-8 of size 8
The buggy address is located 0 bytes to the right of
                8-byte region [ffff8881e68c7ac0, ffff8881e68c7ac8)
[...]

Fixes: 5c42c64f7d54 ("drm/ttm: fix the fix for huge compound pages")
Cc: stable@...r.kernel.org
Signed-off-by: Jann Horn <jannh@...gle.com>
---
I've tested that this makes the KASAN splat go away for me.
I'm not very familiar with the graphics subsystem though,
so it might well be that I've completely misdiagnosed what
the root cause is.

 drivers/gpu/drm/ttm/ttm_page_alloc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index f841accc2c00..68912664a6df 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -731,9 +731,12 @@ static void ttm_put_pages(struct page **pages, unsigned npages, int flags,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 			if (!(flags & TTM_PAGE_FLAG_DMA32)) {
-				for (j = 0; j < HPAGE_PMD_NR; ++j)
+				for (j = 0; j < HPAGE_PMD_NR; ++j) {
+					if (i + j >= npages)
+						break;
 					if (p++ != pages[i + j])
-					    break;
+						break;
+				}
 
 				if (j == HPAGE_PMD_NR)
 					order = HPAGE_PMD_ORDER;
@@ -766,9 +769,12 @@ static void ttm_put_pages(struct page **pages, unsigned npages, int flags,
 			if (!p)
 				break;
 
-			for (j = 0; j < HPAGE_PMD_NR; ++j)
+			for (j = 0; j < HPAGE_PMD_NR; ++j) {
+				if (i + j >= npages)
+					break;
 				if (p++ != pages[i + j])
 				    break;
+			}
 
 			if (j != HPAGE_PMD_NR)
 				break;
-- 
2.21.0.392.gf8f6787159e-goog