Skip to content

Commit d3b277b

Browse files
Pranjal Ramajor Asha Kanojiyaquic-jhugo
authored andcommitted
accel/qaic: Validate user data before grabbing any lock
Validating user data does not need to be protected by any lock and it is safe to move it out of critical region. Fixes: ff13be8 ("accel/qaic: Add datapath") Fixes: 129776a ("accel/qaic: Add control path") Signed-off-by: Pranjal Ramajor Asha Kanojiya <[email protected]> Reviewed-by: Carl Vanderlip <[email protected]> Reviewed-by: Jeffrey Hugo <[email protected]> Signed-off-by: Jeffrey Hugo <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 0e163e5 commit d3b277b

File tree

2 files changed

+27
-46
lines changed

2 files changed

+27
-46
lines changed

drivers/accel/qaic/qaic_control.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,7 +1249,7 @@ static int qaic_manage(struct qaic_device *qdev, struct qaic_user *usr, struct m
12491249

12501250
int qaic_manage_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv)
12511251
{
1252-
struct qaic_manage_msg *user_msg;
1252+
struct qaic_manage_msg *user_msg = data;
12531253
struct qaic_device *qdev;
12541254
struct manage_msg *msg;
12551255
struct qaic_user *usr;
@@ -1258,6 +1258,9 @@ int qaic_manage_ioctl(struct drm_device *dev, void *data, struct drm_file *file_
12581258
int usr_rcu_id;
12591259
int ret;
12601260

1261+
if (user_msg->len > QAIC_MANAGE_MAX_MSG_LENGTH)
1262+
return -EINVAL;
1263+
12611264
usr = file_priv->driver_priv;
12621265

12631266
usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
@@ -1275,13 +1278,6 @@ int qaic_manage_ioctl(struct drm_device *dev, void *data, struct drm_file *file_
12751278
return -ENODEV;
12761279
}
12771280

1278-
user_msg = data;
1279-
1280-
if (user_msg->len > QAIC_MANAGE_MAX_MSG_LENGTH) {
1281-
ret = -EINVAL;
1282-
goto out;
1283-
}
1284-
12851281
msg = kzalloc(QAIC_MANAGE_MAX_MSG_LENGTH + sizeof(*msg), GFP_KERNEL);
12861282
if (!msg) {
12871283
ret = -ENOMEM;

drivers/accel/qaic/qaic_data.c

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,10 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi
663663
if (args->pad)
664664
return -EINVAL;
665665

666+
size = PAGE_ALIGN(args->size);
667+
if (size == 0)
668+
return -EINVAL;
669+
666670
usr = file_priv->driver_priv;
667671
usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
668672
if (!usr->qddev) {
@@ -677,12 +681,6 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi
677681
goto unlock_dev_srcu;
678682
}
679683

680-
size = PAGE_ALIGN(args->size);
681-
if (size == 0) {
682-
ret = -EINVAL;
683-
goto unlock_dev_srcu;
684-
}
685-
686684
bo = qaic_alloc_init_bo();
687685
if (IS_ERR(bo)) {
688686
ret = PTR_ERR(bo);
@@ -936,6 +934,22 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
936934
struct qaic_bo *bo;
937935
int ret;
938936

937+
if (args->hdr.count == 0)
938+
return -EINVAL;
939+
940+
arg_size = args->hdr.count * sizeof(*slice_ent);
941+
if (arg_size / args->hdr.count != sizeof(*slice_ent))
942+
return -EINVAL;
943+
944+
if (args->hdr.size == 0)
945+
return -EINVAL;
946+
947+
if (!(args->hdr.dir == DMA_TO_DEVICE || args->hdr.dir == DMA_FROM_DEVICE))
948+
return -EINVAL;
949+
950+
if (args->data == 0)
951+
return -EINVAL;
952+
939953
usr = file_priv->driver_priv;
940954
usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
941955
if (!usr->qddev) {
@@ -950,43 +964,17 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
950964
goto unlock_dev_srcu;
951965
}
952966

953-
if (args->hdr.count == 0) {
954-
ret = -EINVAL;
955-
goto unlock_dev_srcu;
956-
}
957-
958-
arg_size = args->hdr.count * sizeof(*slice_ent);
959-
if (arg_size / args->hdr.count != sizeof(*slice_ent)) {
960-
ret = -EINVAL;
961-
goto unlock_dev_srcu;
962-
}
963-
964967
if (args->hdr.dbc_id >= qdev->num_dbc) {
965968
ret = -EINVAL;
966969
goto unlock_dev_srcu;
967970
}
968971

969-
if (args->hdr.size == 0) {
970-
ret = -EINVAL;
971-
goto unlock_dev_srcu;
972-
}
973-
974-
if (!(args->hdr.dir == DMA_TO_DEVICE || args->hdr.dir == DMA_FROM_DEVICE)) {
975-
ret = -EINVAL;
976-
goto unlock_dev_srcu;
977-
}
978-
979972
dbc = &qdev->dbc[args->hdr.dbc_id];
980973
if (dbc->usr != usr) {
981974
ret = -EINVAL;
982975
goto unlock_dev_srcu;
983976
}
984977

985-
if (args->data == 0) {
986-
ret = -EINVAL;
987-
goto unlock_dev_srcu;
988-
}
989-
990978
user_data = u64_to_user_ptr(args->data);
991979

992980
slice_ent = kzalloc(arg_size, GFP_KERNEL);
@@ -1316,7 +1304,6 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
13161304
received_ts = ktime_get_ns();
13171305

13181306
size = is_partial ? sizeof(*pexec) : sizeof(*exec);
1319-
13201307
n = (unsigned long)size * args->hdr.count;
13211308
if (args->hdr.count == 0 || n / args->hdr.count != size)
13221309
return -EINVAL;
@@ -1665,6 +1652,9 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
16651652
int rcu_id;
16661653
int ret;
16671654

1655+
if (args->pad != 0)
1656+
return -EINVAL;
1657+
16681658
usr = file_priv->driver_priv;
16691659
usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
16701660
if (!usr->qddev) {
@@ -1679,11 +1669,6 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
16791669
goto unlock_dev_srcu;
16801670
}
16811671

1682-
if (args->pad != 0) {
1683-
ret = -EINVAL;
1684-
goto unlock_dev_srcu;
1685-
}
1686-
16871672
if (args->dbc_id >= qdev->num_dbc) {
16881673
ret = -EINVAL;
16891674
goto unlock_dev_srcu;

0 commit comments

Comments
 (0)