drm/vmwgfx: Protect from excessive execbuf kernel memory allocations
Subsystems (3)
PATCH 1/1 drm/vmwgfx: Protect from excessive execbuf kernel memory allocations
With the new validation code, a malicious user-space app could potentially submit command streams with enough buffer-object and resource references in them to have the resulting allocated validion nodes and relocations make the kernel run out of GFP_KERNEL memory.
Protect from this by having the validation code reserve TTM graphics memory when allocating.
--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 5 +++ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 38 ++++++++++++++++++++++ drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 18 +++++++++- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 37 +++++++++++++++++++++ 6 files changed, 102 insertions(+), 2 deletions(-)
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +3 -1
@@ -49,6 +49,8 @@#define VMWGFX_REPO "In Tree" +#define VMWGFX_VALIDATION_MEM_GRAN (16*PAGE_SIZE) +/** * Fully encoded drm commands. Might move to vmw_drm.h
@@ -918,7 +920,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)spin_unlock (&dev_priv->cap_lock); } - +vmw_validation_mem_init_ttm (dev_priv,VMWGFX_VALIDATION_MEM_GRAN ); ret =vmw_kms_init (dev_priv);if (unlikely (ret !=0 )) goto out_no_kms;
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +5 -0
@@ -606,6 +606,9 @@ struct vmw_private {struct vmw_cmdbuf_man *cman;DECLARE_BITMAP (irqthread_pending,VMW_IRQTHREAD_MAX ); + +/* Validation memory reservation */ +struct vmw_validation_mem vvm; };static inline struct vmw_surface *vmw_res_to_srf (struct vmw_resource *res)
@@ -846,6 +849,8 @@ extern int vmw_ttm_global_init(struct vmw_private *dev_priv);extern void vmw_ttm_global_release (struct vmw_private *dev_priv);extern int vmw_mmap (struct file *filp,struct vm_area_struct *vma); +extern void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv, +size_t gran);/** * TTM buffer object driver - vmwgfx_ttm_buffer.c */
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +2 -0
@@ -3835,6 +3835,8 @@ int vmw_execbuf_process(struct drm_file *file_priv,struct sync_file *sync_file =NULL ;DECLARE_VAL_CONTEXT (val_ctx, &sw_context->res_ht,1 ); +vmw_validation_set_val_mem (&val_ctx, &dev_priv->vvm); +if (flags &DRM_VMW_EXECBUF_FLAG_EXPORT_FENCE_FD ) { out_fence_fd =get_unused_fd_flags (O_CLOEXEC );if (out_fence_fd <0 ) {
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +38 -0
@@ -96,3 +96,41 @@ void vmw_ttm_global_release(struct vmw_private *dev_priv)drm_global_item_unref (&dev_priv->bo_global_ref.ref);drm_global_item_unref (&dev_priv->mem_global_ref); } + +/* struct vmw_validation_mem callback */ +static int vmw_vmt_reserve (struct vmw_validation_mem *m,size_t size) +{ +static struct ttm_operation_ctx ctx = {.interruptible = false, + .no_wait_gpu = false}; + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Reserving %llu\n" , (unsigned long long ) size); +return ttm_mem_global_alloc (vmw_mem_glob (dev_priv), size, &ctx); +} + +/* struct vmw_validation_mem callback */ +static void vmw_vmt_unreserve (struct vmw_validation_mem *m,size_t size) +{ + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Unreserving %llu\n" , (unsigned long long ) size); +return ttm_mem_global_free (vmw_mem_glob (dev_priv), size); +} + +/** +* vmw_validation_mem_init_ttm - Interface the validation memory tracker +* to ttm. +* @dev_priv: Pointer to struct vmw_private. The reason we choose a vmw private +* rather than a struct vmw_validation_mem is to make sure assumption in the +* callbacks that struct vmw_private derives from struct vmw_validation_mem +* holds true. +* @gran: The recommended allocation granularity +*/ +void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv,size_t gran) +{ +struct vmw_validation_mem *vvm = &dev_priv->vvm; + + vvm->reserve_mem = vmw_vmt_reserve; + vvm->unreserve_mem = vmw_vmt_unreserve; + vvm->gran = gran; +}
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c +17 -1
@@ -104,11 +104,25 @@ void *vmw_validation_mem_alloc(struct vmw_validation_context *ctx,return NULL ;if (ctx->mem_size_left < size) { -struct page *page =alloc_page (GFP_KERNEL | __GFP_ZERO); +struct page *page; +if (ctx->vm && ctx->vm_size_left <PAGE_SIZE ) { +int ret = ctx->vm->reserve_mem (ctx->vm, ctx->vm->gran); + +if (ret) +return NULL ; + + ctx->vm_size_left += ctx->vm->gran; + ctx->total_mem += ctx->vm->gran; + } + + page =alloc_page (GFP_KERNEL | __GFP_ZERO);if (!page)return NULL ; +if (ctx->vm) + ctx->vm_size_left -=PAGE_SIZE ; +list_add_tail (&page->lru, &ctx->page_list); ctx->page_address =page_address (page); ctx->mem_size_left =PAGE_SIZE ;
@@ -138,6 +152,8 @@ static void vmw_validation_mem_free(struct vmw_validation_context *ctx) } ctx->mem_size_left =0 ; +if (ctx->vm && ctx->total_mem) + ctx->vm->unreserve_mem (ctx->vm, ctx->total_mem); }/**
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +37 -0
@@ -33,6 +33,21 @@#include <linux/ww_mutex.h> #include <drm/ttm/ttm_execbuf_util.h> +/** +* struct vmw_validation_mem - Custom interface to provide memory reservations +* for the validation code. +* @reserve_mem: Callback to reserve memory +* @unreserve_mem: Callback to unreserve memory +* @gran: Reservation granularity. Contains a hint how much memory should +* be reserved in each call to @reserve_mem(). A slow implementation may want +* reservation to be done in large batches. +*/ +struct vmw_validation_mem { +int (*reserve_mem)(struct vmw_validation_mem *m,size_t size); +void (*unreserve_mem)(struct vmw_validation_mem *m,size_t size); +size_t gran; +}; +/** * struct vmw_validation_context - Per command submission validation context * @ht: Hash table used to find resource- or buffer object duplicates
@@ -47,6 +62,10 @@* buffer objects * @mem_size_left: Free memory left in the last page in @page_list * @page_address: Kernel virtual address of the last page in @page_list +* @vm: A pointer to the memory reservation interface or NULL if no +* memory reservation is needed. +* @vm_size_left: Amount of reserved memory that so far has not been allocated. +* @total_mem: Amount of reserved memory. */ struct vmw_validation_context {struct drm_open_hash *ht;
@@ -59,6 +78,9 @@ struct vmw_validation_context {unsigned int merge_dups;unsigned int mem_size_left;u8 *page_address; +struct vmw_validation_mem *vm; +size_t vm_size_left; +size_t total_mem; };struct vmw_buffer_object ;
@@ -101,6 +123,21 @@ vmw_validation_has_bos(struct vmw_validation_context *ctx)return !list_empty (&ctx->bo_list); } +/** +* vmw_validation_set_val_mem - Register a validation mem object for +* validation memory reservation +* @ctx: The validation context +* @vm: Pointer to a struct vmw_validation_mem +* +* Must be set before the first attempt to allocate validation memory. +*/ +static inline void +vmw_validation_set_val_mem (struct vmw_validation_context *ctx, +struct vmw_validation_mem *vm) +{ + ctx->vm = vm; +} +/** * vmw_validation_set_ht - Register a hash table for duplicate finding * @ctx: The validation context
Signature · 6 lines
2.19.0.rc1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 83 lines · Thomas Hellstrom
With the new validation code, a malicious user-space app could potentially submit command streams with enough buffer-object and resource references in them to have the resulting allocated validion nodes and relocations make the kernel run out of
GFP_KERNELmemory.Protect from this by having the validation code reserve TTM graphics memory when allocating.
Signed-off-by Thomas Hellstrom <[email protected]>--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 5 +++ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 38 ++++++++++++++++++++++ drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 18 +++++++++- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 37 +++++++++++++++++++++ 6 files changed, 102 insertions(+), 2 deletions(-)drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +3
-1@@ -49,6 +49,8 @@#define VMWGFX_REPO "In Tree" +#define VMWGFX_VALIDATION_MEM_GRAN (16*PAGE_SIZE) +/** * Fully encoded drm commands. Might move to vmw_drm.h @@ -918,7 +920,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)spin_unlock (&dev_priv->cap_lock); } - +vmw_validation_mem_init_ttm (dev_priv,VMWGFX_VALIDATION_MEM_GRAN ); ret =vmw_kms_init (dev_priv);if (unlikely (ret !=0 )) goto out_no_kms;drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +5
-0@@ -606,6 +606,9 @@ struct vmw_private {struct vmw_cmdbuf_man *cman;DECLARE_BITMAP (irqthread_pending,VMW_IRQTHREAD_MAX ); + +/* Validation memory reservation */ +struct vmw_validation_mem vvm; };static inline struct vmw_surface *vmw_res_to_srf (struct vmw_resource *res)@@ -846,6 +849,8 @@ extern int vmw_ttm_global_init(struct vmw_private *dev_priv);extern void vmw_ttm_global_release (struct vmw_private *dev_priv);extern int vmw_mmap (struct file *filp,struct vm_area_struct *vma); +extern void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv, +size_t gran);/** * TTM buffer object driver - vmwgfx_ttm_buffer.c */ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +2
-0@@ -3835,6 +3835,8 @@ int vmw_execbuf_process(struct drm_file *file_priv,struct sync_file *sync_file =NULL ;DECLARE_VAL_CONTEXT (val_ctx, &sw_context->res_ht,1 ); +vmw_validation_set_val_mem (&val_ctx, &dev_priv->vvm); +if (flags &DRM_VMW_EXECBUF_FLAG_EXPORT_FENCE_FD ) { out_fence_fd =get_unused_fd_flags (O_CLOEXEC );if (out_fence_fd <0 ) {drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +0
-0@@ -96,3 +96,41 @@ void vmw_ttm_global_release(struct vmw_private *dev_priv)drm_global_item_unref (&dev_priv->bo_global_ref.ref);drm_global_item_unref (&dev_priv->mem_global_ref);
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c void vmw_ttm_global_release(struct vmw_private *dev_priv) · line 107
Quoted 10 lines
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +9
-0} + +/* struct vmw_validation_mem callback */ +static int vmw_vmt_reserve (struct vmw_validation_mem *m,size_t size) +{ +static struct ttm_operation_ctx ctx = {.interruptible = false, + .no_wait_gpu = false}; + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Reserving %llu\n" , (unsigned long long ) size);
Oops. Leftover-debugging printouts. Will be fixed in v2.
Quoted 128 lines
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +29
-0+return ttm_mem_global_alloc (vmw_mem_glob (dev_priv), size, &ctx); +} + +/* struct vmw_validation_mem callback */ +static void vmw_vmt_unreserve (struct vmw_validation_mem *m,size_t size) +{ + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Unreserving %llu\n" , (unsigned long long ) size); +return ttm_mem_global_free (vmw_mem_glob (dev_priv), size); +} + +/** +* vmw_validation_mem_init_ttm - Interface the validation memory tracker +* to ttm. +* @dev_priv: Pointer to struct vmw_private. The reason we choose a vmw private +* rather than a struct vmw_validation_mem is to make sure assumption in the +* callbacks that struct vmw_private derives from struct vmw_validation_mem +* holds true. +* @gran: The recommended allocation granularity +*/ +void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv,size_t gran) +{ +struct vmw_validation_mem *vvm = &dev_priv->vvm; + + vvm->reserve_mem = vmw_vmt_reserve; + vvm->unreserve_mem = vmw_vmt_unreserve; + vvm->gran = gran; +}drivers/gpu/drm/vmwgfx/vmwgfx_validation.c +17
-1@@ -104,11 +104,25 @@ void *vmw_validation_mem_alloc(struct vmw_validation_context *ctx,return NULL ;if (ctx->mem_size_left < size) { -struct page *page =alloc_page (GFP_KERNEL | __GFP_ZERO); +struct page *page; +if (ctx->vm && ctx->vm_size_left <PAGE_SIZE ) { +int ret = ctx->vm->reserve_mem (ctx->vm, ctx->vm->gran); + +if (ret) +return NULL ; + + ctx->vm_size_left += ctx->vm->gran; + ctx->total_mem += ctx->vm->gran; + } + + page =alloc_page (GFP_KERNEL | __GFP_ZERO);if (!page)return NULL ; +if (ctx->vm) + ctx->vm_size_left -=PAGE_SIZE ; +list_add_tail (&page->lru, &ctx->page_list); ctx->page_address =page_address (page); ctx->mem_size_left =PAGE_SIZE ;@@ -138,6 +152,8 @@ static void vmw_validation_mem_free(struct vmw_validation_context *ctx) } ctx->mem_size_left =0 ; +if (ctx->vm && ctx->total_mem) + ctx->vm->unreserve_mem (ctx->vm, ctx->total_mem); }/** drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +30
-0@@ -33,6 +33,21 @@#include <linux/ww_mutex.h> #include <drm/ttm/ttm_execbuf_util.h> +/** +* struct vmw_validation_mem - Custom interface to provide memory reservations +* for the validation code. +* @reserve_mem: Callback to reserve memory +* @unreserve_mem: Callback to unreserve memory +* @gran: Reservation granularity. Contains a hint how much memory should +* be reserved in each call to @reserve_mem(). A slow implementation may want +* reservation to be done in large batches. +*/ +struct vmw_validation_mem { +int (*reserve_mem)(struct vmw_validation_mem *m,size_t size); +void (*unreserve_mem)(struct vmw_validation_mem *m,size_t size); +size_t gran; +}; +/** * struct vmw_validation_context - Per command submission validation context * @ht: Hash table used to find resource- or buffer object duplicates @@ -47,6 +62,10 @@* buffer objects * @mem_size_left: Free memory left in the last page in @page_list * @page_address: Kernel virtual address of the last page in @page_list +* @vm: A pointer to the memory reservation interface or NULL if no +* memory reservation is needed. +* @vm_size_left: Amount of reserved memory that so far has not been allocated. +* @total_mem: Amount of reserved memory. */ struct vmw_validation_context {struct drm_open_hash *ht;@@ -59,6 +78,9 @@ struct vmw_validation_context {unsigned int merge_dups;unsigned int mem_size_left;u8 *page_address; +struct vmw_validation_mem *vm; +size_t vm_size_left; +size_t total_mem; };struct vmw_buffer_object ;@@ -101,6 +123,21 @@ vmw_validation_has_bos(struct vmw_validation_context *ctx)return !list_empty (&ctx->bo_list); } +/** +* vmw_validation_set_val_mem - Register a validation mem object for +* validation memory reservation +* @ctx: The validation context +* @vm: Pointer to a struct vmw_validation_mem +* +* Must be set before the first attempt to allocate validation memory. +*/
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h vmw_validation_has_bos(struct vmw_validation_context *ctx) · line 143
Quoted 10 lines
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +7
-0+static inline void +vmw_validation_set_val_mem (struct vmw_validation_context *ctx, +struct vmw_validation_mem *vm) +{ + ctx->vm = vm; +} +/** * vmw_validation_set_ht - Register a hash table for duplicate finding * @ctx: The validation context
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 11 lines · Thomas Hellstrom
Quoted 9 lines · Thomas Hellstrom
With the new validation code, a malicious user-space app could potentially submit command streams with enough buffer-object and resource references in them to have the resulting allocated validion nodes and relocations make the kernel run out of
GFP_KERNELmemory.Protect from this by having the validation code reserve TTM graphics memory when allocating.
Signed-off-by Thomas Hellstrom <[email protected]>
For protecting GFP_KERNEL memory ... isn't this what cgroups are for? Ioctl will fail with -ENOMEM way before the kernel has actually run out of memory. Including the benefit that it's cross-driver and configurable.
I know that the ttm memory reservations are older than cgroups, so there's good reasons we have them. Just feels like there's better tools available now, which would allow us to provide a better (more consistent across vendors at least) interface to userspace to control the limits.
Cheers, Daniel
Quoted 232 lines
Quoted 74 lines
--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 5 +++ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 38 ++++++++++++++++++++++ drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 18 +++++++++- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 37 +++++++++++++++++++++ 6 files changed, 102 insertions(+), 2 deletions(-)drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +3
-1@@ -49,6 +49,8 @@#define VMWGFX_REPO "In Tree" +#define VMWGFX_VALIDATION_MEM_GRAN (16*PAGE_SIZE) +/** * Fully encoded drm commands. Might move to vmw_drm.h @@ -918,7 +920,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)spin_unlock (&dev_priv->cap_lock); } - +vmw_validation_mem_init_ttm (dev_priv,VMWGFX_VALIDATION_MEM_GRAN ); ret =vmw_kms_init (dev_priv);if (unlikely (ret !=0 )) goto out_no_kms;drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +5
-0@@ -606,6 +606,9 @@ struct vmw_private {struct vmw_cmdbuf_man *cman;DECLARE_BITMAP (irqthread_pending,VMW_IRQTHREAD_MAX ); + +/* Validation memory reservation */ +struct vmw_validation_mem vvm; };static inline struct vmw_surface *vmw_res_to_srf (struct vmw_resource *res)@@ -846,6 +849,8 @@ extern int vmw_ttm_global_init(struct vmw_private *dev_priv);extern void vmw_ttm_global_release (struct vmw_private *dev_priv);extern int vmw_mmap (struct file *filp,struct vm_area_struct *vma); +extern void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv, +size_t gran);/** * TTM buffer object driver - vmwgfx_ttm_buffer.c */ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +2
-0@@ -3835,6 +3835,8 @@ int vmw_execbuf_process(struct drm_file *file_priv,struct sync_file *sync_file =NULL ;DECLARE_VAL_CONTEXT (val_ctx, &sw_context->res_ht,1 ); +vmw_validation_set_val_mem (&val_ctx, &dev_priv->vvm); +if (flags &DRM_VMW_EXECBUF_FLAG_EXPORT_FENCE_FD ) { out_fence_fd =get_unused_fd_flags (O_CLOEXEC );if (out_fence_fd <0 ) {drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +0
-0@@ -96,3 +96,41 @@ void vmw_ttm_global_release(struct vmw_private *dev_priv)drm_global_item_unref (&dev_priv->bo_global_ref.ref);drm_global_item_unref (&dev_priv->mem_global_ref);drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c void vmw_ttm_global_release(struct vmw_private *dev_priv) · line 107
Quoted 10 lines
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +9
-0} + +/* struct vmw_validation_mem callback */ +static int vmw_vmt_reserve (struct vmw_validation_mem *m,size_t size) +{ +static struct ttm_operation_ctx ctx = {.interruptible = false, + .no_wait_gpu = false}; + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Reserving %llu\n" , (unsigned long long ) size);Oops. Leftover-debugging printouts. Will be fixed in v2.
Quoted 128 lines
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +29
-0+return ttm_mem_global_alloc (vmw_mem_glob (dev_priv), size, &ctx); +} + +/* struct vmw_validation_mem callback */ +static void vmw_vmt_unreserve (struct vmw_validation_mem *m,size_t size) +{ + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Unreserving %llu\n" , (unsigned long long ) size); +return ttm_mem_global_free (vmw_mem_glob (dev_priv), size); +} + +/** +* vmw_validation_mem_init_ttm - Interface the validation memory tracker +* to ttm. +* @dev_priv: Pointer to struct vmw_private. The reason we choose a vmw private +* rather than a struct vmw_validation_mem is to make sure assumption in the +* callbacks that struct vmw_private derives from struct vmw_validation_mem +* holds true. +* @gran: The recommended allocation granularity +*/ +void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv,size_t gran) +{ +struct vmw_validation_mem *vvm = &dev_priv->vvm; + + vvm->reserve_mem = vmw_vmt_reserve; + vvm->unreserve_mem = vmw_vmt_unreserve; + vvm->gran = gran; +}drivers/gpu/drm/vmwgfx/vmwgfx_validation.c +17
-1@@ -104,11 +104,25 @@ void *vmw_validation_mem_alloc(struct vmw_validation_context *ctx,return NULL ;if (ctx->mem_size_left < size) { -struct page *page =alloc_page (GFP_KERNEL | __GFP_ZERO); +struct page *page; +if (ctx->vm && ctx->vm_size_left <PAGE_SIZE ) { +int ret = ctx->vm->reserve_mem (ctx->vm, ctx->vm->gran); + +if (ret) +return NULL ; + + ctx->vm_size_left += ctx->vm->gran; + ctx->total_mem += ctx->vm->gran; + } + + page =alloc_page (GFP_KERNEL | __GFP_ZERO);if (!page)return NULL ; +if (ctx->vm) + ctx->vm_size_left -=PAGE_SIZE ; +list_add_tail (&page->lru, &ctx->page_list); ctx->page_address =page_address (page); ctx->mem_size_left =PAGE_SIZE ;@@ -138,6 +152,8 @@ static void vmw_validation_mem_free(struct vmw_validation_context *ctx) } ctx->mem_size_left =0 ; +if (ctx->vm && ctx->total_mem) + ctx->vm->unreserve_mem (ctx->vm, ctx->total_mem); }/** drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +30
-0@@ -33,6 +33,21 @@#include <linux/ww_mutex.h> #include <drm/ttm/ttm_execbuf_util.h> +/** +* struct vmw_validation_mem - Custom interface to provide memory reservations +* for the validation code. +* @reserve_mem: Callback to reserve memory +* @unreserve_mem: Callback to unreserve memory +* @gran: Reservation granularity. Contains a hint how much memory should +* be reserved in each call to @reserve_mem(). A slow implementation may want +* reservation to be done in large batches. +*/ +struct vmw_validation_mem { +int (*reserve_mem)(struct vmw_validation_mem *m,size_t size); +void (*unreserve_mem)(struct vmw_validation_mem *m,size_t size); +size_t gran; +}; +/** * struct vmw_validation_context - Per command submission validation context * @ht: Hash table used to find resource- or buffer object duplicates @@ -47,6 +62,10 @@* buffer objects * @mem_size_left: Free memory left in the last page in @page_list * @page_address: Kernel virtual address of the last page in @page_list +* @vm: A pointer to the memory reservation interface or NULL if no +* memory reservation is needed. +* @vm_size_left: Amount of reserved memory that so far has not been allocated. +* @total_mem: Amount of reserved memory. */ struct vmw_validation_context {struct drm_open_hash *ht;@@ -59,6 +78,9 @@ struct vmw_validation_context {unsigned int merge_dups;unsigned int mem_size_left;u8 *page_address; +struct vmw_validation_mem *vm; +size_t vm_size_left; +size_t total_mem; };struct vmw_buffer_object ;@@ -101,6 +123,21 @@ vmw_validation_has_bos(struct vmw_validation_context *ctx)return !list_empty (&ctx->bo_list); } +/** +* vmw_validation_set_val_mem - Register a validation mem object for +* validation memory reservation +* @ctx: The validation context +* @vm: Pointer to a struct vmw_validation_mem +* +* Must be set before the first attempt to allocate validation memory. +*/ drivers/gpu/drm/vmwgfx/vmwgfx_validation.h vmw_validation_has_bos(struct vmw_validation_context *ctx) · line 143
Quoted 10 lines
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +7
-0+static inline void +vmw_validation_set_val_mem (struct vmw_validation_context *ctx, +struct vmw_validation_mem *vm) +{ + ctx->vm = vm; +} +/** * vmw_validation_set_ht - Register a hash table for duplicate finding * @ctx: The validation context _______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi, Daniel,
Quoted 16 lines · Daniel Vetter
Quoted 10 lines · Thomas Hellstrom
Quoted 9 lines · Thomas Hellstrom
With the new validation code, a malicious user-space app could potentially submit command streams with enough buffer-object and resource references in them to have the resulting allocated validion nodes and relocations make the kernel run out of
GFP_KERNELmemory.Protect from this by having the validation code reserve TTM graphics memory when allocating.
Signed-off-by Thomas Hellstrom <[email protected]>For protecting
GFP_KERNELmemory ... isn't this what cgroups are for? Ioctl will fail with-ENOMEMway before the kernel has actually run out of memory. Including the benefit that it's cross-driver and configurable.
Quoted 7 lines
I know that the ttm memory reservations are older than cgroups, so there's good reasons we have them. Just feels like there's better tools available now, which would allow us to provide a better (more consistent across vendors at least) interface to userspace to control the limits.
Cheers, Daniel
Thanks for the hint.
As the vmwgfx driver is currently completely wired up for the ttm system I'll be using that for this patch, but cgroups is certainly something we could take a deeper look into in the future.
/Thomas
Quoted 223 lines
Quoted 223 lines
Quoted 75 lines
--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 5 +++ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 38 ++++++++++++++++++++++ drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 18 +++++++++- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 37 +++++++++++++++++++++ 6 files changed, 102 insertions(+), 2 deletions(-)drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +3
-1@@ -49,6 +49,8 @@#define VMWGFX_REPO "In Tree" +#define VMWGFX_VALIDATION_MEM_GRAN (16*PAGE_SIZE) +/** * Fully encoded drm commands. Might move to vmw_drm.h @@ -918,7 +920,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)spin_unlock (&dev_priv->cap_lock); } - +vmw_validation_mem_init_ttm (dev_priv,VMWGFX_VALIDATION_MEM_GRAN ); ret =vmw_kms_init (dev_priv);if (unlikely (ret !=0 )) goto out_no_kms;drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +5
-0@@ -606,6 +606,9 @@ struct vmw_private {struct vmw_cmdbuf_man *cman;DECLARE_BITMAP (irqthread_pending,VMW_IRQTHREAD_MAX ); + +/* Validation memory reservation */ +struct vmw_validation_mem vvm; };static inline struct vmw_surface *vmw_res_to_srf (struct vmw_resource *res)@@ -846,6 +849,8 @@ extern int vmw_ttm_global_init(struct vmw_private *dev_priv);extern void vmw_ttm_global_release (struct vmw_private *dev_priv);extern int vmw_mmap (struct file *filp,struct vm_area_struct *vma); +extern void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv, +size_t gran);/** * TTM buffer object driver - vmwgfx_ttm_buffer.c */ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +2
-0@@ -3835,6 +3835,8 @@ int vmw_execbuf_process(struct drm_file *file_priv,struct sync_file *sync_file =NULL ;DECLARE_VAL_CONTEXT (val_ctx, &sw_context->res_ht,1 ); +vmw_validation_set_val_mem (&val_ctx, &dev_priv->vvm); +if (flags &DRM_VMW_EXECBUF_FLAG_EXPORT_FENCE_FD ) { out_fence_fd =get_unused_fd_flags (O_CLOEXEC );if (out_fence_fd <0 ) {drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +0
-0@@ -96,3 +96,41 @@ void vmw_ttm_global_release(struct vmw_private *dev_priv)drm_global_item_unref (&dev_priv->bo_global_ref.ref);drm_global_item_unref (&dev_priv->mem_global_ref);drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c void vmw_ttm_global_release(struct vmw_private *dev_priv) · line 107
Quoted 10 lines
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +9
-0} + +/* struct vmw_validation_mem callback */ +static int vmw_vmt_reserve (struct vmw_validation_mem *m,size_t size) +{ +static struct ttm_operation_ctx ctx = {.interruptible = false, + .no_wait_gpu = false}; + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Reserving %llu\n" , (unsigned long long ) size);Oops. Leftover-debugging printouts. Will be fixed in v2.
Quoted 135 lines
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c +29
-0+return ttm_mem_global_alloc (vmw_mem_glob (dev_priv), size, &ctx); +} + +/* struct vmw_validation_mem callback */ +static void vmw_vmt_unreserve (struct vmw_validation_mem *m,size_t size) +{ + struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm); + +DRM_INFO ("Unreserving %llu\n" , (unsigned long long ) size); +return ttm_mem_global_free (vmw_mem_glob (dev_priv), size); +} + +/** +* vmw_validation_mem_init_ttm - Interface the validation memory tracker +* to ttm. +* @dev_priv: Pointer to struct vmw_private. The reason we choose a vmw private +* rather than a struct vmw_validation_mem is to make sure assumption in the +* callbacks that struct vmw_private derives from struct vmw_validation_mem +* holds true. +* @gran: The recommended allocation granularity +*/ +void vmw_validation_mem_init_ttm (struct vmw_private *dev_priv,size_t gran) +{ +struct vmw_validation_mem *vvm = &dev_priv->vvm; + + vvm->reserve_mem = vmw_vmt_reserve; + vvm->unreserve_mem = vmw_vmt_unreserve; + vvm->gran = gran; +}drivers/gpu/drm/vmwgfx/vmwgfx_validation.c +17
-1@@ -104,11 +104,25 @@ void *vmw_validation_mem_alloc(struct vmw_validation_context *ctx,return NULL ;if (ctx->mem_size_left < size) { -struct page *page =alloc_page (GFP_KERNEL | __GFP_ZERO); +struct page *page; +if (ctx->vm && ctx->vm_size_left <PAGE_SIZE ) { +int ret = ctx->vm->reserve_mem (ctx->vm, ctx->vm->gran); + +if (ret) +return NULL ; + + ctx->vm_size_left += ctx->vm->gran; + ctx->total_mem += ctx->vm->gran; + } + + page =alloc_page (GFP_KERNEL | __GFP_ZERO);if (!page)return NULL ; +if (ctx->vm) + ctx->vm_size_left -=PAGE_SIZE ; +list_add_tail (&page->lru, &ctx->page_list); ctx->page_address =page_address (page); ctx->mem_size_left =PAGE_SIZE ;@@ -138,6 +152,8 @@ static void vmw_validation_mem_free(struct vmw_validation_context *ctx) } ctx->mem_size_left =0 ; +if (ctx->vm && ctx->total_mem) + ctx->vm->unreserve_mem (ctx->vm, ctx->total_mem); }/** drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +37
-0@@ -33,6 +33,21 @@#include <linux/ww_mutex.h> #include <drm/ttm/ttm_execbuf_util.h> +/** +* struct vmw_validation_mem - Custom interface to provide memory reservations +* for the validation code. +* @reserve_mem: Callback to reserve memory +* @unreserve_mem: Callback to unreserve memory +* @gran: Reservation granularity. Contains a hint how much memory should +* be reserved in each call to @reserve_mem(). A slow implementation may want +* reservation to be done in large batches. +*/ +struct vmw_validation_mem { +int (*reserve_mem)(struct vmw_validation_mem *m,size_t size); +void (*unreserve_mem)(struct vmw_validation_mem *m,size_t size); +size_t gran; +}; +/** * struct vmw_validation_context - Per command submission validation context * @ht: Hash table used to find resource- or buffer object duplicates @@ -47,6 +62,10 @@* buffer objects * @mem_size_left: Free memory left in the last page in @page_list * @page_address: Kernel virtual address of the last page in @page_list +* @vm: A pointer to the memory reservation interface or NULL if no +* memory reservation is needed. +* @vm_size_left: Amount of reserved memory that so far has not been allocated. +* @total_mem: Amount of reserved memory. */ struct vmw_validation_context {struct drm_open_hash *ht;@@ -59,6 +78,9 @@ struct vmw_validation_context {unsigned int merge_dups;unsigned int mem_size_left;u8 *page_address; +struct vmw_validation_mem *vm; +size_t vm_size_left; +size_t total_mem; };struct vmw_buffer_object ;@@ -101,6 +123,21 @@ vmw_validation_has_bos(struct vmw_validation_context *ctx)return !list_empty (&ctx->bo_list); } +/** +* vmw_validation_set_val_mem - Register a validation mem object for +* validation memory reservation +* @ctx: The validation context +* @vm: Pointer to a struct vmw_validation_mem +* +* Must be set before the first attempt to allocate validation memory. +*/ +static inline void +vmw_validation_set_val_mem (struct vmw_validation_context *ctx, +struct vmw_validation_mem *vm) +{ + ctx->vm = vm; +} +
Quoted 10 lines
Quoted 8 lines
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h vmw_validation_has_bos(struct vmw_validation_context *ctx) · line 143
Quoted 3 lines
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +0
-0/** * vmw_validation_set_ht - Register a hash table for duplicate finding * @ctx: The validation context _______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel