qxl_process_single_command() copies relocation entries from userspace and
passes them to apply_reloc() / apply_surf_reloc() without checking
whether dst_offset falls within the destination buffer object.
apply_reloc() writes 8 bytes and apply_surf_reloc() writes 4 bytes at
the byte offset named by the relocation. A userspace-chosen offset that
exceeds the BO's allocation leads to an out-of-bounds write into
adjacent slab memory.
When the destination is the command/release BO itself (dst_handle == 0),
the write lands relative to the release's own release_info header. An
offset of zero overwrites the release_info.id, which the garbage
collector later uses as a release index, leading to a use-after-free of
a release chosen by userspace. Both paths are reachable from any render
client (DRM_AUTH).
Add two bounds checks:
- When writing into the release's own BO (dst_handle == 0), require
that dst_offset points past the release_info header and that the
final byte (offset + write width) stays within the command data area.
- For every relocation, require that the final byte offset plus the
write width does not exceed the destination BO size. Cast to u64
before the addition so the check cannot be bypassed by a 32-bit
integer overflow on ILP32 platforms.
Fixes: f64122c1f6ad ("drm: add qxl driver.")
Cc: [email protected]
Signed-off-by: Aldo Ariel Panzardo <[email protected]>
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026ce..e727a35c9 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -204,6 +204,7 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
for (i = 0; i < cmd->relocs_num; ++i) {
struct drm_qxl_reloc reloc;
struct drm_qxl_reloc __user *u = u64_to_user_ptr(cmd->relocs);
+ size_t write_size;
if (copy_from_user(&reloc, u + i, sizeof(reloc))) {
ret = -EFAULT;
@@ -220,6 +221,9 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
}
reloc_info[i].type = reloc.reloc_type;
+ write_size = reloc.reloc_type == QXL_RELOC_TYPE_BO ?
+ sizeof(uint64_t) : sizeof(uint32_t);
+
if (reloc.dst_handle) {
ret = qxlhw_handle_to_bo(file_priv, reloc.dst_handle,
release,
&reloc_info[i].dst_bo);
@@ -227,10 +231,22 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
goto out_free_bos;
reloc_info[i].dst_offset = reloc.dst_offset;
} else {
+ if (reloc.dst_offset < sizeof(union qxl_release_info) ||
+ reloc.dst_offset + write_size >
+ sizeof(union qxl_release_info) + cmd->command_size)
{
+ ret = -EINVAL;
+ goto out_free_bos;
+ }
reloc_info[i].dst_bo = cmd_bo;
reloc_info[i].dst_offset = reloc.dst_offset +
release->release_offset;
}
+ if ((u64)reloc_info[i].dst_offset + write_size >
+ reloc_info[i].dst_bo->tbo.base.size) {
+ ret = -EINVAL;
+ goto out_free_bos;
+ }
+
/* reserve and validate the reloc dst bo */
if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle) {
ret = qxlhw_handle_to_bo(file_priv, reloc.src_handle,
release,
--
2.43.0
