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: dd08ebf6c352 ("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]>
(cherry picked from commit
e5283bd4dfecbd3335f43b62a68e24dae23f59e4)
Signed-off-by: Thomas Hellström <[email protected]>
#include "xe_reg_whitelist.h"
#include "xe_rtp_types.h"
#include "xe_reg_whitelist.h"
#include "xe_rtp_types.h"
-#define XE_REG_SR_GROW_STEP_DEFAULT 16
-
static void reg_sr_fini(struct drm_device *drm, void *arg)
{
struct xe_reg_sr *sr = arg;
static void reg_sr_fini(struct drm_device *drm, void *arg)
{
struct xe_reg_sr *sr = arg;
+ struct xe_reg_sr_entry *entry;
+ unsigned long reg;
+
+ xa_for_each(&sr->xa, reg, entry)
+ kfree(entry);
- kfree(sr->pool.arr);
- memset(&sr->pool, 0, sizeof(sr->pool));
}
int xe_reg_sr_init(struct xe_reg_sr *sr, const char *name, struct xe_device *xe)
{
xa_init(&sr->xa);
}
int xe_reg_sr_init(struct xe_reg_sr *sr, const char *name, struct xe_device *xe)
{
xa_init(&sr->xa);
- memset(&sr->pool, 0, sizeof(sr->pool));
- sr->pool.grow_step = XE_REG_SR_GROW_STEP_DEFAULT;
sr->name = name;
return drmm_add_action_or_reset(&xe->drm, reg_sr_fini, sr);
}
EXPORT_SYMBOL_IF_KUNIT(xe_reg_sr_init);
sr->name = name;
return drmm_add_action_or_reset(&xe->drm, reg_sr_fini, sr);
}
EXPORT_SYMBOL_IF_KUNIT(xe_reg_sr_init);
-static struct xe_reg_sr_entry *alloc_entry(struct xe_reg_sr *sr)
-{
- if (sr->pool.used == sr->pool.allocated) {
- struct xe_reg_sr_entry *arr;
-
- arr = krealloc_array(sr->pool.arr,
- ALIGN(sr->pool.allocated + 1, sr->pool.grow_step),
- sizeof(*arr), GFP_KERNEL);
- if (!arr)
- return NULL;
-
- sr->pool.arr = arr;
- sr->pool.allocated += sr->pool.grow_step;
- }
-
- return &sr->pool.arr[sr->pool.used++];
-}
-
static bool compatible_entries(const struct xe_reg_sr_entry *e1,
const struct xe_reg_sr_entry *e2)
{
static bool compatible_entries(const struct xe_reg_sr_entry *e1,
const struct xe_reg_sr_entry *e2)
{
- pentry = alloc_entry(sr);
+ pentry = kmalloc(sizeof(*pentry), GFP_KERNEL);
if (!pentry) {
ret = -ENOMEM;
goto fail;
if (!pentry) {
ret = -ENOMEM;
goto fail;
- struct {
- struct xe_reg_sr_entry *arr;
- unsigned int used;
- unsigned int allocated;
- unsigned int grow_step;
- } pool;
struct xarray xa;
const char *name;
struct xarray xa;
const char *name;