Skip to content

Commit 157261c

Browse files
author
Thomas Hellström
committed
drm/xe/pt: Allow for stricter type- and range checking
Distinguish between xe_pt and the xe_pt_dir subclass when allocating and freeing. Also use a fixed-size array for the xe_pt_dir page entries to make life easier for dynamic range- checkers. Finally rename the page-directory child pointer array to "children". While no functional change, this fixes ubsan splats similar to: [ 51.463021] ------------[ cut here ]------------ [ 51.463022] UBSAN: array-index-out-of-bounds in drivers/gpu/drm/xe/xe_pt.c:47:9 [ 51.463023] index 0 is out of range for type 'xe_ptw *[*]' [ 51.463024] CPU: 5 PID: 2778 Comm: xe_vm Tainted: G U 6.8.0-rc1+ #218 [ 51.463026] Hardware name: ASUS System Product Name/PRIME B560M-A AC, BIOS 2001 02/01/2023 [ 51.463027] Call Trace: [ 51.463028] <TASK> [ 51.463029] dump_stack_lvl+0x47/0x60 [ 51.463030] __ubsan_handle_out_of_bounds+0x95/0xd0 [ 51.463032] xe_pt_destroy+0xa5/0x150 [xe] [ 51.463088] __xe_pt_unbind_vma+0x36c/0x9b0 [xe] [ 51.463144] xe_vm_unbind+0xd8/0x580 [xe] [ 51.463204] ? drm_exec_prepare_obj+0x3f/0x60 [drm_exec] [ 51.463208] __xe_vma_op_execute+0x5da/0x910 [xe] [ 51.463268] ? __drm_gpuvm_sm_unmap+0x1cb/0x220 [drm_gpuvm] [ 51.463272] ? radix_tree_node_alloc.constprop.0+0x89/0xc0 [ 51.463275] ? drm_gpuva_it_remove+0x1f3/0x2a0 [drm_gpuvm] [ 51.463279] ? drm_gpuva_remove+0x2f/0xc0 [drm_gpuvm] [ 51.463283] xe_vm_bind_ioctl+0x1a55/0x20b0 [xe] [ 51.463344] ? __pfx_xe_vm_bind_ioctl+0x10/0x10 [xe] [ 51.463414] drm_ioctl_kernel+0xb6/0x120 [ 51.463416] drm_ioctl+0x287/0x4e0 [ 51.463418] ? __pfx_xe_vm_bind_ioctl+0x10/0x10 [xe] [ 51.463481] __x64_sys_ioctl+0x94/0xd0 [ 51.463484] do_syscall_64+0x86/0x170 [ 51.463486] ? syscall_exit_to_user_mode+0x7d/0x200 [ 51.463488] ? do_syscall_64+0x96/0x170 [ 51.463490] ? do_syscall_64+0x96/0x170 [ 51.463492] entry_SYSCALL_64_after_hwframe+0x6e/0x76 [ 51.463494] RIP: 0033:0x7f246bfe817d [ 51.463498] Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8 64 48 2b 04 25 28 00 00 00 [ 51.463501] RSP: 002b:00007ffc1bd19ad0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 51.463502] RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f246bfe817d [ 51.463504] RDX: 00007ffc1bd19b60 RSI: 0000000040886445 RDI: 0000000000000003 [ 51.463505] RBP: 00007ffc1bd19b20 R08: 0000000000000000 R09: 0000000000000000 [ 51.463506] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc1bd19b60 [ 51.463508] R13: 0000000040886445 R14: 0000000000000003 R15: 0000000000010000 [ 51.463510] </TASK> [ 51.463517] ---[ end trace ]--- v2 - Fix kerneldoc warning (Matthew Brost) Fixes: dd08ebf ("drm/xe: Introduce a new DRM driver for Intel GPUs") Cc: Rodrigo Vivi <[email protected]> Cc: Matthew Brost <[email protected]> Signed-off-by: Thomas Hellström <[email protected]> Reviewed-by: Matthew Brost <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent f8237c8 commit 157261c

File tree

3 files changed

+29
-31
lines changed

3 files changed

+29
-31
lines changed

drivers/gpu/drm/xe/xe_pt.c

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020

2121
struct xe_pt_dir {
2222
struct xe_pt pt;
23-
/** @dir: Directory structure for the xe_pt_walk functionality */
24-
struct xe_ptw_dir dir;
23+
/** @children: Array of page-table child nodes */
24+
struct xe_ptw *children[XE_PDES];
2525
};
2626

2727
#if IS_ENABLED(CONFIG_DRM_XE_DEBUG_VM)
@@ -44,7 +44,7 @@ static struct xe_pt_dir *as_xe_pt_dir(struct xe_pt *pt)
4444

4545
static struct xe_pt *xe_pt_entry(struct xe_pt_dir *pt_dir, unsigned int index)
4646
{
47-
return container_of(pt_dir->dir.entries[index], struct xe_pt, base);
47+
return container_of(pt_dir->children[index], struct xe_pt, base);
4848
}
4949

5050
static u64 __xe_pt_empty_pte(struct xe_tile *tile, struct xe_vm *vm,
@@ -65,6 +65,14 @@ static u64 __xe_pt_empty_pte(struct xe_tile *tile, struct xe_vm *vm,
6565
XE_PTE_NULL;
6666
}
6767

68+
static void xe_pt_free(struct xe_pt *pt)
69+
{
70+
if (pt->level)
71+
kfree(as_xe_pt_dir(pt));
72+
else
73+
kfree(pt);
74+
}
75+
6876
/**
6977
* xe_pt_create() - Create a page-table.
7078
* @vm: The vm to create for.
@@ -85,15 +93,19 @@ struct xe_pt *xe_pt_create(struct xe_vm *vm, struct xe_tile *tile,
8593
{
8694
struct xe_pt *pt;
8795
struct xe_bo *bo;
88-
size_t size;
8996
int err;
9097

91-
size = !level ? sizeof(struct xe_pt) : sizeof(struct xe_pt_dir) +
92-
XE_PDES * sizeof(struct xe_ptw *);
93-
pt = kzalloc(size, GFP_KERNEL);
98+
if (level) {
99+
struct xe_pt_dir *dir = kzalloc(sizeof(*dir), GFP_KERNEL);
100+
101+
pt = (dir) ? &dir->pt : NULL;
102+
} else {
103+
pt = kzalloc(sizeof(*pt), GFP_KERNEL);
104+
}
94105
if (!pt)
95106
return ERR_PTR(-ENOMEM);
96107

108+
pt->level = level;
97109
bo = xe_bo_create_pin_map(vm->xe, tile, vm, SZ_4K,
98110
ttm_bo_type_kernel,
99111
XE_BO_CREATE_VRAM_IF_DGFX(tile) |
@@ -106,8 +118,7 @@ struct xe_pt *xe_pt_create(struct xe_vm *vm, struct xe_tile *tile,
106118
goto err_kfree;
107119
}
108120
pt->bo = bo;
109-
pt->level = level;
110-
pt->base.dir = level ? &as_xe_pt_dir(pt)->dir : NULL;
121+
pt->base.children = level ? as_xe_pt_dir(pt)->children : NULL;
111122

112123
if (vm->xef)
113124
xe_drm_client_add_bo(vm->xef->client, pt->bo);
@@ -116,7 +127,7 @@ struct xe_pt *xe_pt_create(struct xe_vm *vm, struct xe_tile *tile,
116127
return pt;
117128

118129
err_kfree:
119-
kfree(pt);
130+
xe_pt_free(pt);
120131
return ERR_PTR(err);
121132
}
122133

@@ -193,7 +204,7 @@ void xe_pt_destroy(struct xe_pt *pt, u32 flags, struct llist_head *deferred)
193204
deferred);
194205
}
195206
}
196-
kfree(pt);
207+
xe_pt_free(pt);
197208
}
198209

199210
/**
@@ -358,7 +369,7 @@ xe_pt_insert_entry(struct xe_pt_stage_bind_walk *xe_walk, struct xe_pt *parent,
358369
struct iosys_map *map = &parent->bo->vmap;
359370

360371
if (unlikely(xe_child))
361-
parent->base.dir->entries[offset] = &xe_child->base;
372+
parent->base.children[offset] = &xe_child->base;
362373

363374
xe_pt_write(xe_walk->vm->xe, map, offset, pte);
364375
parent->num_live++;
@@ -853,7 +864,7 @@ static void xe_pt_commit_bind(struct xe_vma *vma,
853864
xe_pt_destroy(xe_pt_entry(pt_dir, j_),
854865
xe_vma_vm(vma)->flags, deferred);
855866

856-
pt_dir->dir.entries[j_] = &newpte->base;
867+
pt_dir->children[j_] = &newpte->base;
857868
}
858869
kfree(entries[i].pt_entries);
859870
}
@@ -1506,7 +1517,7 @@ xe_pt_commit_unbind(struct xe_vma *vma,
15061517
xe_pt_destroy(xe_pt_entry(pt_dir, i),
15071518
xe_vma_vm(vma)->flags, deferred);
15081519

1509-
pt_dir->dir.entries[i] = NULL;
1520+
pt_dir->children[i] = NULL;
15101521
}
15111522
}
15121523
}

drivers/gpu/drm/xe/xe_pt_walk.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ int xe_pt_walk_range(struct xe_ptw *parent, unsigned int level,
7474
u64 addr, u64 end, struct xe_pt_walk *walk)
7575
{
7676
pgoff_t offset = xe_pt_offset(addr, level, walk);
77-
struct xe_ptw **entries = parent->dir ? parent->dir->entries : NULL;
77+
struct xe_ptw **entries = parent->children ? parent->children : NULL;
7878
const struct xe_pt_walk_ops *ops = walk->ops;
7979
enum page_walk_action action;
8080
struct xe_ptw *child;

drivers/gpu/drm/xe/xe_pt_walk.h

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,15 @@
88
#include <linux/pagewalk.h>
99
#include <linux/types.h>
1010

11-
struct xe_ptw_dir;
12-
1311
/**
1412
* struct xe_ptw - base class for driver pagetable subclassing.
15-
* @dir: Pointer to an array of children if any.
13+
* @children: Pointer to an array of children if any.
1614
*
1715
* Drivers could subclass this, and if it's a page-directory, typically
18-
* embed the xe_ptw_dir::entries array in the same allocation.
16+
* embed an array of xe_ptw pointers.
1917
*/
2018
struct xe_ptw {
21-
struct xe_ptw_dir *dir;
22-
};
23-
24-
/**
25-
* struct xe_ptw_dir - page directory structure
26-
* @entries: Array holding page directory children.
27-
*
28-
* It is the responsibility of the user to ensure @entries is
29-
* correctly sized.
30-
*/
31-
struct xe_ptw_dir {
32-
struct xe_ptw *entries[0];
19+
struct xe_ptw **children;
3320
};
3421

3522
/**

0 commit comments

Comments
 (0)