On Tue, 27 Aug 2024, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> Replace the fake VLA at end of the vbva_mouse_pointer_shape shape with
> a real VLA to fix a "memcpy: detected field-spanning write error" warning:
>
> [ 13.319813] memcpy: detected field-spanning write (size 16896) of single field "p->data" at drivers/gpu/drm/vboxvideo/hgsmi_base.c:154 (size 4)
> [ 13.319841] WARNING: CPU: 0 PID: 1105 at drivers/gpu/drm/vboxvideo/hgsmi_base.c:154 hgsmi_update_pointer_shape+0x192/0x1c0 [vboxvideo]
> [ 13.320038] Call Trace:
> [ 13.320173] hgsmi_update_pointer_shape [vboxvideo]
> [ 13.320184] vbox_cursor_atomic_update [vboxvideo]
>
> Note as mentioned in the added comment it seems the original length
> calculation for the allocated and send hgsmi buffer is 4 bytes too large.
> Changing this is not the goal of this patch, so this behavior is kept.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> Resend, can I get an ack from someone to push this to drm-misc-next please ?
>
> (this has been tested with a vboxsvga adapter in a vbox vm)
> ---
> drivers/gpu/drm/vboxvideo/hgsmi_base.c | 10 +++++++++-
> drivers/gpu/drm/vboxvideo/vboxvideo.h | 4 +---
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vboxvideo/hgsmi_base.c b/drivers/gpu/drm/vboxvideo/hgsmi_base.c
> index 8c041d7ce4f1..87dccaecc3e5 100644
> --- a/drivers/gpu/drm/vboxvideo/hgsmi_base.c
> +++ b/drivers/gpu/drm/vboxvideo/hgsmi_base.c
> @@ -139,7 +139,15 @@ int hgsmi_update_pointer_shape(struct gen_pool *ctx, u32 flags,
> flags |= VBOX_MOUSE_POINTER_VISIBLE;
> }
>
> - p = hgsmi_buffer_alloc(ctx, sizeof(*p) + pixel_len, HGSMI_CH_VBVA,
> + /*
> + * The 4 extra bytes come from switching struct vbva_mouse_pointer_shape
> + * from having a 4 bytes fixed array at the end to using a proper VLA
> + * at the end. These 4 extra bytes were not subtracted from sizeof(*p)
> + * before the switch to the VLA, so this way the behavior is unchanged.
> + * Chances are these 4 extra bytes are not necessary but they are kept
> + * to avoid regressions.
> + */
> + p = hgsmi_buffer_alloc(ctx, sizeof(*p) + pixel_len + 4, HGSMI_CH_VBVA,
Nitpick, struct_size(p, data, pixel_len + 4) is a better choice for VLAs
than open coding.
Either way, and FWIW,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
BR,
Jani.
> VBVA_MOUSE_POINTER_SHAPE);
> if (!p)
> return -ENOMEM;
> diff --git a/drivers/gpu/drm/vboxvideo/vboxvideo.h b/drivers/gpu/drm/vboxvideo/vboxvideo.h
> index f60d82504da0..79ec8481de0e 100644
> --- a/drivers/gpu/drm/vboxvideo/vboxvideo.h
> +++ b/drivers/gpu/drm/vboxvideo/vboxvideo.h
> @@ -351,10 +351,8 @@ struct vbva_mouse_pointer_shape {
> * Bytes in the gap between the AND and the XOR mask are undefined.
> * XOR mask scanlines have no gap between them and size of XOR mask is:
> * xor_len = width * 4 * height.
> - *
> - * Preallocate 4 bytes for accessing actual data as p->data.
> */
> - u8 data[4];
> + u8 data[];
> } __packed;
>
> /* pointer is visible */
--
Jani Nikula, Intel
