On 9/7/25 23:40, Dan Carpenter wrote:
Hello Lizhi Hou,
Commit 2f509fe6a42c ("accel/amdxdna: Add ioctl
DRM_IOCTL_AMDXDNA_GET_ARRAY") from Sep 2, 2025 (linux-next), leads to
the following (UNPUBLISHED) Smatch static checker warning:
drivers/accel/amdxdna/aie2_pci.c:904 aie2_query_ctx_status_array()
warn: potential user controlled sizeof overflow
'args->num_element * args->element_size' '1-u32max(user) *
1-u32max(user)'
drivers/accel/amdxdna/aie2_pci.c
891 static int aie2_query_ctx_status_array(struct amdxdna_client *client,
892 struct amdxdna_drm_get_array
*args)
893 {
894 struct amdxdna_drm_get_array array_args;
895 struct amdxdna_dev *xdna = client->xdna;
896 struct amdxdna_client *tmp_client;
897 int ret;
898
899 drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
900
901 array_args.element_size = min(args->element_size,
902 sizeof(struct
amdxdna_drm_hwctx_entry));
Instead of min() here we should just return -EINVAL if they are !=.
The request element_size from runtime tools can be smaller or bigger
than sizeof(struct amdxdna_drm_hwctx_entry).
If element_size is smaller, element_size bytes will be copied to user space.
If it is bigger, sizeof(struct amdxdna_drm_hwctx_entry) bytes will be
copied.
And the actual element size and number of element will be returned to
userspace.
903 array_args.buffer = args->buffer;
--> 904 array_args.num_element = args->num_element * args->element_size
/
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
These are both u32 values controlled by the user so this is an integer
overflow bug. Security bug.
This will not cause an issue. array_args.num_element is considered as
user control as well.
If it is too big, the actual number of active hwctx will be returned.
It is better to put a reasonable limitation. I would add a check
(args->num_element < 1K && args->element_size < 4K). Will this fix the
smatch warning?
Thanks,
Lizhi
905 array_args.element_size;
906 list_for_each_entry(tmp_client, &xdna->client_list, node) {
907 ret = amdxdna_hwctx_walk(tmp_client, &array_args,
908 aie2_hwctx_status_cb);
909 if (ret)
910 break;
911 }
912
913 args->element_size = array_args.element_size;
914 args->num_element = (u32)((array_args.buffer - args->buffer) /
915 args->element_size);
916
917 return ret;
918 }
regards,
dan carpenter
