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
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
dri-devel@lists.freedesktop.org
-
Dan Carpenter -
Yat Sin, David