* [bug report] drm/amdkfd: CRIU checkpoint and restore queue mqds
@ 2022-02-18 7:29 Dan Carpenter
2022-02-18 13:18 ` Yat Sin, David
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-02-18 7:29 UTC (permalink / raw)
To: david.yatsin; +Cc: dri-devel, amd-gfx
Hello David Yat Sin,
The patch 42c6c48214b7: "drm/amdkfd: CRIU checkpoint and restore
queue mqds" from Jan 25, 2021, leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v9.c:344 restore_mqd()
error: 'ctl_stack_size' from user is not capped properly
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
762 int kfd_criu_restore_queue(struct kfd_process *p,
763 uint8_t __user *user_priv_ptr,
764 uint64_t *priv_data_offset,
765 uint64_t max_priv_data_size)
766 {
767 uint8_t *mqd, *ctl_stack, *q_extra_data = NULL;
768 struct kfd_criu_queue_priv_data *q_data;
769 struct kfd_process_device *pdd;
770 uint64_t q_extra_data_size;
771 struct queue_properties qp;
772 unsigned int queue_id;
773 int ret = 0;
774
775 if (*priv_data_offset + sizeof(*q_data) > max_priv_data_size)
776 return -EINVAL;
777
778 q_data = kmalloc(sizeof(*q_data), GFP_KERNEL);
779 if (!q_data)
780 return -ENOMEM;
781
782 ret = copy_from_user(q_data, user_priv_ptr + *priv_data_offset, sizeof(*q_data));
783 if (ret) {
784 ret = -EFAULT;
785 goto exit;
786 }
787
788 *priv_data_offset += sizeof(*q_data);
789 q_extra_data_size = q_data->ctl_stack_size + q_data->mqd_size;
^^^^^^^^^^^^^^^^^^^^^^
ctl_stack_size comes from the user a couple lines earlier. It's a u32
and so is q_data->mqd_size. This addition can have an integer overflow.
790
791 if (*priv_data_offset + q_extra_data_size > max_priv_data_size) {'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Which means that this limit check doesn't work.
792 ret = -EINVAL;
793 goto exit;
794 }
795
796 q_extra_data = kmalloc(q_extra_data_size, GFP_KERNEL);
797 if (!q_extra_data) {
798 ret = -ENOMEM;
799 goto exit;
800 }
801
802 ret = copy_from_user(q_extra_data, user_priv_ptr + *priv_data_offset, q_extra_data_size);
803 if (ret) {
804 ret = -EFAULT;
805 goto exit;
806 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [bug report] drm/amdkfd: CRIU checkpoint and restore queue mqds
2022-02-18 7:29 [bug report] drm/amdkfd: CRIU checkpoint and restore queue mqds Dan Carpenter
@ 2022-02-18 13:18 ` Yat Sin, David
0 siblings, 0 replies; 2+ messages in thread
From: Yat Sin, David @ 2022-02-18 13:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel, amd-gfx
Hi Dan,
Thank you for catching this. I will look into it and post a patch.
Regards,
David
-----Original Message-----
From: Dan Carpenter <dan.carpenter@oracle.com>
Sent: Friday, February 18, 2022 2:30 AM
To: Yat Sin, David <David.YatSin@amd.com>
Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: [bug report] drm/amdkfd: CRIU checkpoint and restore queue mqds
Hello David Yat Sin,
The patch 42c6c48214b7: "drm/amdkfd: CRIU checkpoint and restore queue mqds" from Jan 25, 2021, leads to the following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v9.c:344 restore_mqd()
error: 'ctl_stack_size' from user is not capped properly
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
762 int kfd_criu_restore_queue(struct kfd_process *p,
763 uint8_t __user *user_priv_ptr,
764 uint64_t *priv_data_offset,
765 uint64_t max_priv_data_size)
766 {
767 uint8_t *mqd, *ctl_stack, *q_extra_data = NULL;
768 struct kfd_criu_queue_priv_data *q_data;
769 struct kfd_process_device *pdd;
770 uint64_t q_extra_data_size;
771 struct queue_properties qp;
772 unsigned int queue_id;
773 int ret = 0;
774
775 if (*priv_data_offset + sizeof(*q_data) > max_priv_data_size)
776 return -EINVAL;
777
778 q_data = kmalloc(sizeof(*q_data), GFP_KERNEL);
779 if (!q_data)
780 return -ENOMEM;
781
782 ret = copy_from_user(q_data, user_priv_ptr + *priv_data_offset, sizeof(*q_data));
783 if (ret) {
784 ret = -EFAULT;
785 goto exit;
786 }
787
788 *priv_data_offset += sizeof(*q_data);
789 q_extra_data_size = q_data->ctl_stack_size + q_data->mqd_size;
^^^^^^^^^^^^^^^^^^^^^^ ctl_stack_size comes from the user a couple lines earlier. It's a u32 and so is q_data->mqd_size. This addition can have an integer overflow.
790
791 if (*priv_data_offset + q_extra_data_size > max_priv_data_size) {'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Which means that this limit check doesn't work.
792 ret = -EINVAL;
793 goto exit;
794 }
795
796 q_extra_data = kmalloc(q_extra_data_size, GFP_KERNEL);
797 if (!q_extra_data) {
798 ret = -ENOMEM;
799 goto exit;
800 }
801
802 ret = copy_from_user(q_extra_data, user_priv_ptr + *priv_data_offset, q_extra_data_size);
803 if (ret) {
804 ret = -EFAULT;
805 goto exit;
806 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-18 13:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-18 7:29 [bug report] drm/amdkfd: CRIU checkpoint and restore queue mqds Dan Carpenter
2022-02-18 13:18 ` Yat Sin, David
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.