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@xxxxxxxxxx>
Sent: Friday, February 18, 2022 2:30 AM
To: Yat Sin, David <David.YatSin@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
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
