mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 11:03:07 -04:00
drm/amdkfd: hold event_mutex while checkpointing CRIU events
kfd_criu_checkpoint_events() counts the entries in p->event_idr via
kfd_get_num_events(), allocates an array sized to that count, and then
walks the same IDR to fill it. Neither the count nor the walk holds
p->event_mutex.
The CRIU checkpoint caller holds only p->mutex. Event create and destroy
(kfd_event_create()/kfd_event_destroy()) take p->event_mutex and do not
take p->mutex, so a second thread in the same process can insert or remove
events between the count and the walk. If an event is inserted, the walk
iterates more entries than were counted and writes past the end of the
ev_privs allocation; if an event is removed, the walk dereferences an
entry that is being freed.
Hold p->event_mutex across the count and the walk so both observe a
consistent view of p->event_idr. The lock is released before
copy_to_user(), which only touches the local buffer. The caller already
holds p->mutex and the create/destroy paths never take p->mutex, so the
p->mutex -> p->event_mutex order is not inverted and no deadlock is
introduced.
Fixes: 40e8a766a7 ("drm/amdkfd: CRIU checkpoint and restore events")
Signed-off-by: William Palacek <William.Palacek@amd.com>
Reviewed-by: Alysa Liu <Alysa.Liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
This commit is contained in:
committed by
Alex Deucher
parent
218c0b4c5b
commit
ff57e223ab
@@ -476,15 +476,27 @@ int kfd_criu_checkpoint_events(struct kfd_process *p,
|
||||
int ret = 0;
|
||||
struct kfd_event *ev;
|
||||
uint32_t ev_id;
|
||||
uint32_t num_events;
|
||||
|
||||
uint32_t num_events = kfd_get_num_events(p);
|
||||
/* Serialize the count and the walk below against concurrent event
|
||||
* create/destroy. Those paths take only p->event_mutex, not the
|
||||
* p->mutex held by the CRIU checkpoint caller, so without this the
|
||||
* event_idr can grow between kfd_get_num_events() and the loop and the
|
||||
* walk writes past the ev_privs allocation.
|
||||
*/
|
||||
mutex_lock(&p->event_mutex);
|
||||
|
||||
if (!num_events)
|
||||
num_events = kfd_get_num_events(p);
|
||||
if (!num_events) {
|
||||
mutex_unlock(&p->event_mutex);
|
||||
return 0;
|
||||
}
|
||||
|
||||
ev_privs = kvzalloc(num_events * sizeof(*ev_privs), GFP_KERNEL);
|
||||
if (!ev_privs)
|
||||
if (!ev_privs) {
|
||||
mutex_unlock(&p->event_mutex);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
|
||||
idr_for_each_entry(&p->event_idr, ev, ev_id) {
|
||||
@@ -525,6 +537,8 @@ int kfd_criu_checkpoint_events(struct kfd_process *p,
|
||||
i++;
|
||||
}
|
||||
|
||||
mutex_unlock(&p->event_mutex);
|
||||
|
||||
ret = copy_to_user(user_priv_data + *priv_data_offset,
|
||||
ev_privs, num_events * sizeof(*ev_privs));
|
||||
if (ret) {
|
||||
|
||||
Reference in New Issue
Block a user