On Mon May 25, 2026 at 9:56 AM EDT, David Francis wrote:
This may be a dumb question, but the use of the * means "restore_id" is
dereferenced. If the conditions are evaluated, either will result in
restore_id being dereferenced, even if both conditions are false. Then
can *restore_id still be safely dereferenced in the idr_alloc call?
restore_id is a normal pointer to an int. Dereferencing it should have no side
effects.
So this is perfectly safe
David Francis
Ok. (I tested this myself too with a simple C program).
And I see above the condition is a check for p->signal_page, so:
Reviewed-by: Joshua Peisach <[email protected]>
________________________________________
From: Joshua Peisach <[email protected]>
Sent: Thursday, May 21, 2026 9:35 AM
To: Francis, David; [email protected]
Cc: amd-gfx
Subject: Re: [PATCH] drm/amdkfd: Check bounds in
allocate_event_notification_slot
On Thu May 21, 2026 at 9:28 AM EDT, David Francis wrote:
The valid event ids go from 0 to signal_mapped_size / 8
(usually 256).
allocate_event_notification_slot has an option to specify
an event id to allocate at, used by CRIU. We weren't checking
the bounds on that value.
Check them.
Signed-off-by: David Francis <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_events.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index e9be798c0a2b..5a4fe68a7986 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -107,6 +107,9 @@ static int allocate_event_notification_slot(struct
kfd_process *p,
}
if (restore_id) {
+ if (*restore_id < 0 || *restore_id >= p->signal_mapped_size / 8)
+ return -EINVAL;
+
id = idr_alloc(&p->event_idr, ev, *restore_id, *restore_id + 1,
GFP_KERNEL);
} else {
This may be a dumb question, but the use of the * means "restore_id" is
dereferenced. If the conditions are evaluated, either will result in
restore_id being dereferenced, even if both conditions are false. Then
can *restore_id still be safely dereferenced in the idr_alloc call?
