Fix out-of-bounds accesses and missing bounds checks in the QXL display
and dirty-framebuffer drawing paths:
1. In qxl_create_cursor() and qxl_cursor_atomic_check(), verify that the
cursor GEM BO is at least 64 * 64 * 4 bytes (16 KiB) before copying
into the QXL cursor command, and zero-initialize the stack struct
qxl_cursor header.
2. In qxl_draw_dirty_fb() and qxl_framebuffer_surface_dirty(), validate
clip coordinates against the framebuffer dimensions and negative x/y
offsets before computing pixel pointers, and handle odd clip counts
when DRM_MODE_FB_DIRTY_ANNOTATE_COPY is set.
3. In qxl_display_copy_rom_client_monitors_config(), read
qdev->rom->client_monitors_config.count once with READ_ONCE() and
clamp num_monitors to ARRAY_SIZE(heads).
4. In qxl_prepare_shadow(), propagate qxl_bo_create() allocation failures
to qxl_plane_prepare_fb().
Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Fixes: b4b27f08f9f9 ("drm/qxl: rework cursor plane")
Assisted-by: LLM
Signed-off-by: Hui Peng <[email protected]>
---
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 0719fc6a52d5..0fc14312e027 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -82,7 +82,7 @@ static int qxl_display_copy_rom_client_monitors_config(struct
qxl_device *qdev)
uint32_t crc;
int status = MONITORS_CONFIG_UNCHANGED;
- num_monitors = qdev->rom->client_monitors_config.count;
+ num_monitors = READ_ONCE(qdev->rom->client_monitors_config.count);
crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
sizeof(qdev->rom->client_monitors_config));
if (crc != qdev->rom->client_monitors_config_crc)
@@ -95,9 +95,9 @@ static int qxl_display_copy_rom_client_monitors_config(struct
qxl_device *qdev)
DRM_DEBUG_KMS("client monitors list will be truncated: %d <
%d\n",
qxl_num_crtc, num_monitors);
num_monitors = qxl_num_crtc;
- } else {
- num_monitors = qdev->rom->client_monitors_config.count;
}
+ if (num_monitors > ARRAY_SIZE(qdev->rom->client_monitors_config.heads))
+ num_monitors =
ARRAY_SIZE(qdev->rom->client_monitors_config.heads);
if (qdev->client_monitors_config
&& (num_monitors != qdev->client_monitors_config->count)) {
status = MONITORS_CONFIG_MODIFIED;
@@ -454,6 +454,9 @@ static int qxl_framebuffer_surface_dirty(struct
drm_framebuffer *fb,
norect.x2 = fb->width;
norect.y2 = fb->height;
} else if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY) {
+ if (num_clips < 2)
+ goto out_lock_end;
+ clips++;
num_clips /= 2;
inc = 2; /* skip source rects */
}
@@ -601,7 +604,7 @@ static struct qxl_bo *qxl_create_cursor(struct qxl_device
*qdev,
struct qxl_cursor cursor;
int ret;
- if (!user_bo)
+ if (!user_bo || user_bo->tbo.base.size < size)
return NULL;
ret = qxl_bo_create(qdev, sizeof(struct qxl_cursor) + size,
@@ -618,6 +621,7 @@ static struct qxl_bo *qxl_create_cursor(struct qxl_device
*qdev,
if (ret)
goto err_unmap;
+ memset(&cursor, 0, sizeof(cursor));
cursor.header.unique = 0;
cursor.header.type = SPICE_CURSOR_TYPE_ALPHA;
cursor.header.width = 64;
@@ -819,10 +823,11 @@ static void qxl_calc_dumb_shadow(struct qxl_device *qdev,
DRM_DEBUG("%dx%d\n", surf->width, surf->height);
}
-static void qxl_prepare_shadow(struct qxl_device *qdev, struct qxl_bo *user_bo,
- int crtc_index)
+static int qxl_prepare_shadow(struct qxl_device *qdev, struct qxl_bo *user_bo,
+ int crtc_index)
{
struct qxl_surface surf;
+ int ret;
qxl_update_dumb_head(qdev, crtc_index,
user_bo);
@@ -836,9 +841,11 @@ static void qxl_prepare_shadow(struct qxl_device *qdev,
struct qxl_bo *user_bo,
(&qdev->dumb_shadow_bo->tbo.base);
qdev->dumb_shadow_bo = NULL;
}
- qxl_bo_create(qdev, surf.height * surf.stride,
- true, true, QXL_GEM_DOMAIN_SURFACE, 0,
- &surf, &qdev->dumb_shadow_bo);
+ ret = qxl_bo_create(qdev, surf.height * surf.stride,
+ true, true, QXL_GEM_DOMAIN_SURFACE, 0,
+ &surf, &qdev->dumb_shadow_bo);
+ if (ret)
+ return ret;
}
if (user_bo->shadow != qdev->dumb_shadow_bo) {
if (user_bo->shadow) {
@@ -851,6 +858,7 @@ static void qxl_prepare_shadow(struct qxl_device *qdev,
struct qxl_bo *user_bo,
user_bo->shadow = qdev->dumb_shadow_bo;
qxl_bo_pin(user_bo->shadow);
}
+ return 0;
}
static int qxl_plane_prepare_fb(struct drm_plane *plane,
@@ -869,7 +877,9 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
user_bo->is_dumb) {
- qxl_prepare_shadow(qdev, user_bo, new_state->crtc->index);
+ ret = qxl_prepare_shadow(qdev, user_bo, new_state->crtc->index);
+ if (ret)
+ return ret;
}
if (plane->type == DRM_PLANE_TYPE_CURSOR &&
@@ -919,7 +929,25 @@ static const uint32_t qxl_cursor_plane_formats[] = {
DRM_FORMAT_ARGB8888,
};
+static int qxl_cursor_atomic_check(struct drm_plane *plane,
+ struct drm_atomic_commit *state)
+{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
+
plane);
+ struct drm_framebuffer *fb = new_plane_state->fb;
+
+ if (!fb)
+ return 0;
+
+ if (fb->width != 64 || fb->height != 64 ||
+ !fb->obj[0] || fb->obj[0]->size < 64 * 64 * 4)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct drm_plane_helper_funcs qxl_cursor_helper_funcs = {
+ .atomic_check = qxl_cursor_atomic_check,
.atomic_update = qxl_cursor_atomic_update,
.atomic_disable = qxl_cursor_atomic_disable,
.prepare_fb = qxl_plane_prepare_fb,
diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
index 3a3e127ce297..302c17d87d1c 100644
--- a/drivers/gpu/drm/qxl/qxl_draw.c
+++ b/drivers/gpu/drm/qxl/qxl_draw.c
@@ -154,27 +154,40 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
struct qxl_drm_image *dimage;
int ret;
- ret = alloc_drawable(qdev, &release);
- if (ret)
- return;
-
- clips->x1 += dumb_shadow_offset;
- clips->x2 += dumb_shadow_offset;
-
- left = clips->x1;
- right = clips->x2;
+ left = clips->x1 + dumb_shadow_offset;
+ right = clips->x2 + dumb_shadow_offset;
top = clips->y1;
bottom = clips->y2;
/* skip the first clip rect */
for (i = 1, clips_ptr = clips + inc;
i < num_clips; i++, clips_ptr += inc) {
- left = min_t(int, left, (int)clips_ptr->x1);
- right = max_t(int, right, (int)clips_ptr->x2);
+ left = min_t(int, left, (int)clips_ptr->x1 +
dumb_shadow_offset);
+ right = max_t(int, right, (int)clips_ptr->x2 +
dumb_shadow_offset);
top = min_t(int, top, (int)clips_ptr->y1);
bottom = max_t(int, bottom, (int)clips_ptr->y2);
}
+ if (dumb_shadow_offset < 0 || dumb_shadow_offset > INT_MAX - fb->width)
+ return;
+
+ left = clamp_t(int, left, dumb_shadow_offset,
+ dumb_shadow_offset + fb->width);
+ right = clamp_t(int, right, dumb_shadow_offset,
+ dumb_shadow_offset + fb->width);
+ top = clamp_t(int, top, 0, fb->height);
+ bottom = clamp_t(int, bottom, 0, fb->height);
+
+ if (left >= right || top >= bottom)
+ return;
+
+ if ((size_t)bottom * stride > fb->obj[0]->size)
+ return;
+
+ ret = alloc_drawable(qdev, &release);
+ if (ret)
+ return;
+
width = right - left;
height = bottom - top;
