diff mbox series

drm/vmwgfx: Limit display layout ioctl array size to VMWGFX_NUM_DISPLAY_UNITS

Message ID 20240808200634.1074083-1-ian.forbes@broadcom.com (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: Limit display layout ioctl array size to VMWGFX_NUM_DISPLAY_UNITS | expand

Commit Message

Ian Forbes Aug. 8, 2024, 8:06 p.m. UTC 
Currently the array size is only limited by the largest kmalloc size which
is incorrect. This change will also return a more specific error message
than ENOMEM to userspace.

Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 3 ---
 3 files changed, 5 insertions(+), 6 deletions(-)

Comments

Zack Rusin Aug. 13, 2024, 5:06 p.m. UTC | #1 
On Thu, Aug 8, 2024 at 4:06 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> Currently the array size is only limited by the largest kmalloc size which
> is incorrect. This change will also return a more specific error message
> than ENOMEM to userspace.
>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 3 ---
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 32f50e595809..888349f2aac1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -62,7 +62,7 @@
>  #define VMWGFX_DRIVER_MINOR 20
>  #define VMWGFX_DRIVER_PATCHLEVEL 0
>  #define VMWGFX_FIFO_STATIC_SIZE (1024*1024)
> -#define VMWGFX_MAX_DISPLAYS 16
> +#define VMWGFX_NUM_DISPLAY_UNITS 8
>  #define VMWGFX_CMD_BOUNCE_INIT_SIZE 32768
>
>  #define VMWGFX_MIN_INITIAL_WIDTH 1280
> @@ -82,7 +82,7 @@
>  #define VMWGFX_NUM_GB_CONTEXT 256
>  #define VMWGFX_NUM_GB_SHADER 20000
>  #define VMWGFX_NUM_GB_SURFACE 32768
> -#define VMWGFX_NUM_GB_SCREEN_TARGET VMWGFX_MAX_DISPLAYS
> +#define VMWGFX_NUM_GB_SCREEN_TARGET VMWGFX_NUM_DISPLAY_UNITS
>  #define VMWGFX_NUM_DXCONTEXT 256
>  #define VMWGFX_NUM_DXQUERY 512
>  #define VMWGFX_NUM_MOB (VMWGFX_NUM_GB_CONTEXT +\
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 288ed0bb75cb..884804274dfb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2225,7 +2225,7 @@ int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
>         struct drm_mode_config *mode_config = &dev->mode_config;
>         struct drm_vmw_update_layout_arg *arg =
>                 (struct drm_vmw_update_layout_arg *)data;
> -       void __user *user_rects;
> +       const void __user *user_rects;
>         struct drm_vmw_rect *rects;
>         struct drm_rect *drm_rects;
>         unsigned rects_size;
> @@ -2237,6 +2237,8 @@ int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
>                                             VMWGFX_MIN_INITIAL_HEIGHT};
>                 vmw_du_update_layout(dev_priv, 1, &def_rect);
>                 return 0;
> +       } else if (arg->num_outputs > VMWGFX_NUM_DISPLAY_UNITS) {
> +               return -E2BIG;
>         }
>
>         rects_size = arg->num_outputs * sizeof(struct drm_vmw_rect);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 6141fadf81ef..2a6c6d6581e0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -199,9 +199,6 @@ struct vmw_kms_dirty {
>         s32 unit_y2;
>  };
>
> -#define VMWGFX_NUM_DISPLAY_UNITS 8
> -
> -
>  #define vmw_framebuffer_to_vfb(x) \
>         container_of(x, struct vmw_framebuffer, base)
>  #define vmw_framebuffer_to_vfbs(x) \
> --
> 2.34.1
>

Looks good. Thanks.

Reviewed-by: Zack Rusin <zack.rusin@broadcom.com>

z
Martin Krastev Aug. 19, 2024, 2:24 p.m. UTC | #2 
On Thu, Aug 8, 2024 at 11:06 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> Currently the array size is only limited by the largest kmalloc size which
> is incorrect. This change will also return a more specific error message
> than ENOMEM to userspace.
>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 3 ---
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 32f50e595809..888349f2aac1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -62,7 +62,7 @@
>  #define VMWGFX_DRIVER_MINOR 20
>  #define VMWGFX_DRIVER_PATCHLEVEL 0
>  #define VMWGFX_FIFO_STATIC_SIZE (1024*1024)
> -#define VMWGFX_MAX_DISPLAYS 16
> +#define VMWGFX_NUM_DISPLAY_UNITS 8
>  #define VMWGFX_CMD_BOUNCE_INIT_SIZE 32768
>
>  #define VMWGFX_MIN_INITIAL_WIDTH 1280
> @@ -82,7 +82,7 @@
>  #define VMWGFX_NUM_GB_CONTEXT 256
>  #define VMWGFX_NUM_GB_SHADER 20000
>  #define VMWGFX_NUM_GB_SURFACE 32768
> -#define VMWGFX_NUM_GB_SCREEN_TARGET VMWGFX_MAX_DISPLAYS
> +#define VMWGFX_NUM_GB_SCREEN_TARGET VMWGFX_NUM_DISPLAY_UNITS
>  #define VMWGFX_NUM_DXCONTEXT 256
>  #define VMWGFX_NUM_DXQUERY 512
>  #define VMWGFX_NUM_MOB (VMWGFX_NUM_GB_CONTEXT +\
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 288ed0bb75cb..884804274dfb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2225,7 +2225,7 @@ int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
>         struct drm_mode_config *mode_config = &dev->mode_config;
>         struct drm_vmw_update_layout_arg *arg =
>                 (struct drm_vmw_update_layout_arg *)data;
> -       void __user *user_rects;
> +       const void __user *user_rects;
>         struct drm_vmw_rect *rects;
>         struct drm_rect *drm_rects;
>         unsigned rects_size;
> @@ -2237,6 +2237,8 @@ int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
>                                             VMWGFX_MIN_INITIAL_HEIGHT};
>                 vmw_du_update_layout(dev_priv, 1, &def_rect);
>                 return 0;
> +       } else if (arg->num_outputs > VMWGFX_NUM_DISPLAY_UNITS) {
> +               return -E2BIG;
>         }
>
>         rects_size = arg->num_outputs * sizeof(struct drm_vmw_rect);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 6141fadf81ef..2a6c6d6581e0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -199,9 +199,6 @@ struct vmw_kms_dirty {
>         s32 unit_y2;
>  };
>
> -#define VMWGFX_NUM_DISPLAY_UNITS 8
> -
> -
>  #define vmw_framebuffer_to_vfb(x) \
>         container_of(x, struct vmw_framebuffer, base)
>  #define vmw_framebuffer_to_vfbs(x) \
> --
> 2.34.1
>

LGTM.

Reviewed-by: Martin Krastev <martin.krastev@broadcom.com>

Regards,
Martin
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 32f50e595809..888349f2aac1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -62,7 +62,7 @@ 
 #define VMWGFX_DRIVER_MINOR 20
 #define VMWGFX_DRIVER_PATCHLEVEL 0
 #define VMWGFX_FIFO_STATIC_SIZE (1024*1024)
-#define VMWGFX_MAX_DISPLAYS 16
+#define VMWGFX_NUM_DISPLAY_UNITS 8
 #define VMWGFX_CMD_BOUNCE_INIT_SIZE 32768
 
 #define VMWGFX_MIN_INITIAL_WIDTH 1280
@@ -82,7 +82,7 @@ 
 #define VMWGFX_NUM_GB_CONTEXT 256
 #define VMWGFX_NUM_GB_SHADER 20000
 #define VMWGFX_NUM_GB_SURFACE 32768
-#define VMWGFX_NUM_GB_SCREEN_TARGET VMWGFX_MAX_DISPLAYS
+#define VMWGFX_NUM_GB_SCREEN_TARGET VMWGFX_NUM_DISPLAY_UNITS
 #define VMWGFX_NUM_DXCONTEXT 256
 #define VMWGFX_NUM_DXQUERY 512
 #define VMWGFX_NUM_MOB (VMWGFX_NUM_GB_CONTEXT +\
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 288ed0bb75cb..884804274dfb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2225,7 +2225,7 @@  int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct drm_vmw_update_layout_arg *arg =
 		(struct drm_vmw_update_layout_arg *)data;
-	void __user *user_rects;
+	const void __user *user_rects;
 	struct drm_vmw_rect *rects;
 	struct drm_rect *drm_rects;
 	unsigned rects_size;
@@ -2237,6 +2237,8 @@  int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
 					    VMWGFX_MIN_INITIAL_HEIGHT};
 		vmw_du_update_layout(dev_priv, 1, &def_rect);
 		return 0;
+	} else if (arg->num_outputs > VMWGFX_NUM_DISPLAY_UNITS) {
+		return -E2BIG;
 	}
 
 	rects_size = arg->num_outputs * sizeof(struct drm_vmw_rect);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 6141fadf81ef..2a6c6d6581e0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -199,9 +199,6 @@  struct vmw_kms_dirty {
 	s32 unit_y2;
 };
 
-#define VMWGFX_NUM_DISPLAY_UNITS 8
-
-
 #define vmw_framebuffer_to_vfb(x) \
 	container_of(x, struct vmw_framebuffer, base)
 #define vmw_framebuffer_to_vfbs(x) \