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: 40e8a766a761 ("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>
(cherry picked from commit ff57e223ab105795b05d3ef3f3c35a5a441bcbaa)
Cc: stable@vger.kernel.org

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 78abfc0f77809a3e15f9d68b001f8a1958cf5e3d..f705c61bdb1df047a37b3a5c81664d9a99725ec3 100644 (file)
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -548,15 +548,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) {
@@ -597,6 +609,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) {