Hi Triet, The code looks right to me, but the commit message does not say why the change is needed - and that is what stalled the same change before:
https://lore.kernel.org/all/[email protected]/ > Are you sure of this? The list should be freed in rocket_job_close(), > just before freeing the entity. Fair against that patch, which added kfree(rocket_priv->scheds) while keeping kfree(entity->sched_list) - a double free for num_cores > 1. Yours replaces the free instead, so it avoids that. The missing argument is why kfree(entity->sched_list) is not already enough. drm_sched_entity_init() stores the list conditionally: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; With one scheduler it keeps only entity->rq and drops the array pointer, and rocket_job_open() has already let its own copy go out of scope. So for num_cores == 1 the kfree() in rocket_job_close() is a kfree(NULL) and the array leaks unreachably. For num_cores > 1 there is no leak, which is why this is invisible in normal use on RK3588. num_cores == 1 is reachable there anyway: rocket_probe() registers the DRM device when the *first* core binds and increments num_cores as the others follow, so an open() racing the probe of cores 1 and 2 gets a single-scheduler entity. rocket_remove() also decrements num_cores with the device still registered. Independent of the leak, the driver reads a pointer back out of drm_sched's own struct field to free it, and what lands in that field is drm_sched's decision. Owning the allocation removes that dependency - that seems worth stating on its own. Two small things: - No Fixes: tag, although this fixes a leak. Same one as 1/2 fits. - kfree() still runs before drm_sched_entity_destroy(), so entity->sched_list dangles across teardown. Not a live UAF today (select_rq() is only reached from drm_sched_job_arm(), and .postclose runs after the last fd reference is gone), but the order is backwards and moving it after destroy costs nothing. Caveat: I could not measure the leak - no kmemleak or KASAN here, and with three cores I cannot open the num_cores == 1 window on purpose. The above is from the source. What I did verify is that this plus the rocket_job_open() half of 1/2, rebased on drm-misc-next, builds clean and does not regress normal submits or the malformed inputs I tried. Nit: prefix should be accel/rocket:, and "Changses" below the ---. Thanks, Sidong
