Commit d7b02865 authored by Lucas De Marchi's avatar Lucas De Marchi Committed by Thomas Hellström
Browse files

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: dd08ebf6 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241209232739.147417-2-lucas.demarchi@intel.com


Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
(cherry picked from commit e5283bd4)
Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
parent cefade70

drivers/gpu/drm/xe/xe_reg_sr.c

+6 −25
Original line number Diff line number Diff line
@@ -27,46 +27,27 @@ 
#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;

	struct xe_reg_sr_entry *entry;

	unsigned long reg;



	xa_for_each(&sr->xa, reg, entry)

		kfree(entry);



	xa_destroy(&sr->xa);

	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);

	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);



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)

{
@@ -112,7 +93,7 @@ int xe_reg_sr_add(struct xe_reg_sr *sr, 
		return 0;

	}



	pentry = alloc_entry(sr);

	pentry = kmalloc(sizeof(*pentry), GFP_KERNEL);

	if (!pentry) {

		ret = -ENOMEM;

		goto fail;

drivers/gpu/drm/xe/xe_reg_sr_types.h

+0 −6
Original line number Diff line number Diff line
@@ -20,12 +20,6 @@ struct xe_reg_sr_entry { 
};



struct xe_reg_sr {

	struct {

		struct xe_reg_sr_entry *arr;

		unsigned int used;

		unsigned int allocated;

		unsigned int grow_step;

	} pool;

	struct xarray xa;

	const char *name;