[Spice-devel] [PATCH win-qxl] miniport: fix invalid memory access from previous patch

[Alon Levy]

On Wed, Aug 08, 2012 at 08:13:29PM +0200, Marc-André Lureau wrote:
> The patch 253b781773190afef313390542f2d68995e302d7 implementing custom
> display resolution is accessing unowned memory regions.

ACK with or without split.

> 
> Interestingly, the driver worked fine on Windows XP but BSOD on Win7.
> ---
>  miniport/qxl.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/miniport/qxl.c b/miniport/qxl.c
> index 44c2a40..003669b 100644
> --- a/miniport/qxl.c
> +++ b/miniport/qxl.c
> @@ -600,6 +600,7 @@ VP_STATUS InitModes(QXLExtension *dev)
>          return ERROR_INVALID_DATA;
>      }
>  
> +    n_modes += 2;

So this is basically the fix (allocate the two extra modes) and the rest
is cleanup.. would be nice to split.

>  #if (WINVER < 0x0501) //Win2K
>      error = VideoPortAllocateBuffer(dev, n_modes * sizeof(VIDEO_MODE_INFORMATION), &modes_info);
>  
> @@ -614,8 +615,8 @@ VP_STATUS InitModes(QXLExtension *dev)
>          return ERROR_NOT_ENOUGH_MEMORY;
>      }
>  #endif
> -    VideoPortZeroMemory(modes_info, sizeof(VIDEO_MODE_INFORMATION) * n_modes + 2);
> -    for (i = 0; i < n_modes; i++) {
> +    VideoPortZeroMemory(modes_info, sizeof(VIDEO_MODE_INFORMATION) * n_modes);
> +    for (i = 0; i < modes->n_modes; i++) {
>          error = SetVideoModeInfo(dev, &modes_info[i], &modes->modes[i]);
>          if (error != NO_ERROR) {
>              VideoPortFreePool(dev, modes_info);
> @@ -627,13 +628,14 @@ VP_STATUS InitModes(QXLExtension *dev)
>      /* 2 dummy modes for custom display resolution */
>      /* This is necessary to bypass Windows mode index check, that
>         would prevent reusing the same index */
> -    dev->custom_mode = n_modes;
> -    memcpy(&modes_info[n_modes], &modes_info[0], sizeof(VIDEO_MODE_INFORMATION));
> -    modes_info[n_modes].ModeIndex = n_modes;
> -    memcpy(&modes_info[n_modes + 1], &modes_info[0], sizeof(VIDEO_MODE_INFORMATION));
> -    modes_info[n_modes + 1].ModeIndex = n_modes + 1;
> +    dev->custom_mode = modes->n_modes;
>  
> -    dev->n_modes = n_modes + 2;
> +    for (i = dev->custom_mode; i <= dev->custom_mode + 1; ++i) {
> +        memcpy(&modes_info[i], &modes_info[0], sizeof(VIDEO_MODE_INFORMATION));
> +        modes_info[i].ModeIndex = i;
> +    }
> +
> +    dev->n_modes = n_modes;
>      dev->modes = modes_info;
>      DEBUG_PRINT((dev, 0, "%s OK\n", __FUNCTION__));
>      return NO_ERROR;
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel