Skip to content

Commit e5283bd

Browse files
committed
drm/xe/reg_sr: Remove register pool
That pool implementation doesn't really work: if the krealloc happens to move the memory and return another address, the entries in the xarray become invalid, leading to use-after-free later: BUG: KASAN: slab-use-after-free in xe_reg_sr_apply_mmio+0x570/0x760 [xe] Read of size 4 at addr ffff8881244b2590 by task modprobe/2753 Allocated by task 2753: kasan_save_stack+0x39/0x70 kasan_save_track+0x14/0x40 kasan_save_alloc_info+0x37/0x60 __kasan_kmalloc+0xc3/0xd0 __kmalloc_node_track_caller_noprof+0x200/0x6d0 krealloc_noprof+0x229/0x380 Simplify the code to fix the bug. A better pooling strategy may be added back later if needed. Fixes: dd08ebf ("drm/xe: Introduce a new DRM driver for Intel GPUs") Reviewed-by: Matt Roper <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Lucas De Marchi <[email protected]>
1 parent 6533863 commit e5283bd

File tree

2 files changed

+6
-31
lines changed

2 files changed

+6
-31
lines changed

drivers/gpu/drm/xe/xe_reg_sr.c

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,46 +27,27 @@
2727
#include "xe_reg_whitelist.h"
2828
#include "xe_rtp_types.h"
2929

30-
#define XE_REG_SR_GROW_STEP_DEFAULT 16
31-
3230
static void reg_sr_fini(struct drm_device *drm, void *arg)
3331
{
3432
struct xe_reg_sr *sr = arg;
33+
struct xe_reg_sr_entry *entry;
34+
unsigned long reg;
35+
36+
xa_for_each(&sr->xa, reg, entry)
37+
kfree(entry);
3538

3639
xa_destroy(&sr->xa);
37-
kfree(sr->pool.arr);
38-
memset(&sr->pool, 0, sizeof(sr->pool));
3940
}
4041

4142
int xe_reg_sr_init(struct xe_reg_sr *sr, const char *name, struct xe_device *xe)
4243
{
4344
xa_init(&sr->xa);
44-
memset(&sr->pool, 0, sizeof(sr->pool));
45-
sr->pool.grow_step = XE_REG_SR_GROW_STEP_DEFAULT;
4645
sr->name = name;
4746

4847
return drmm_add_action_or_reset(&xe->drm, reg_sr_fini, sr);
4948
}
5049
EXPORT_SYMBOL_IF_KUNIT(xe_reg_sr_init);
5150

52-
static struct xe_reg_sr_entry *alloc_entry(struct xe_reg_sr *sr)
53-
{
54-
if (sr->pool.used == sr->pool.allocated) {
55-
struct xe_reg_sr_entry *arr;
56-
57-
arr = krealloc_array(sr->pool.arr,
58-
ALIGN(sr->pool.allocated + 1, sr->pool.grow_step),
59-
sizeof(*arr), GFP_KERNEL);
60-
if (!arr)
61-
return NULL;
62-
63-
sr->pool.arr = arr;
64-
sr->pool.allocated += sr->pool.grow_step;
65-
}
66-
67-
return &sr->pool.arr[sr->pool.used++];
68-
}
69-
7051
static bool compatible_entries(const struct xe_reg_sr_entry *e1,
7152
const struct xe_reg_sr_entry *e2)
7253
{
@@ -112,7 +93,7 @@ int xe_reg_sr_add(struct xe_reg_sr *sr,
11293
return 0;
11394
}
11495

115-
pentry = alloc_entry(sr);
96+
pentry = kmalloc(sizeof(*pentry), GFP_KERNEL);
11697
if (!pentry) {
11798
ret = -ENOMEM;
11899
goto fail;

drivers/gpu/drm/xe/xe_reg_sr_types.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@ struct xe_reg_sr_entry {
2020
};
2121

2222
struct xe_reg_sr {
23-
struct {
24-
struct xe_reg_sr_entry *arr;
25-
unsigned int used;
26-
unsigned int allocated;
27-
unsigned int grow_step;
28-
} pool;
2923
struct xarray xa;
3024
const char *name;
3125

0 commit comments

Comments
 (0)