Re: [PATCH 3/3] drm/amd: validate user GEM object size

2018-12-21 Thread kbuild test robot
Hi Yu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.20-rc7 next-20181221]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Yu-Zhao/drm-amd-fix-race-in-page-flip-job/20181222-072937
config: x86_64-randconfig-s5-12221153 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/cdev.h:8:0,
from include/drm/drmP.h:36,
from drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:26:
   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c: In function 
'amdgpu_display_user_framebuffer_create':
   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:556:28: warning: format '%d' 
expects argument of type 'int', but argument 4 has type 'size_t {aka long 
unsigned int}' [-Wformat=]
  dev_err(>pdev->dev, "Invalid GEM size: expecting %d but got %d\n",
   ^
   include/linux/device.h:1370:22: note: in definition of macro 'dev_fmt'
#define dev_fmt(fmt) fmt
 ^~~
>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:556:3: note: in expansion of 
>> macro 'dev_err'
  dev_err(>pdev->dev, "Invalid GEM size: expecting %d but got %d\n",
  ^~~

vim +/dev_err +556 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c

   521  
   522  struct drm_framebuffer *
   523  amdgpu_display_user_framebuffer_create(struct drm_device *dev,
   524 struct drm_file *file_priv,
   525 const struct drm_mode_fb_cmd2 
*mode_cmd)
   526  {
   527  struct drm_gem_object *obj;
   528  struct amdgpu_framebuffer *amdgpu_fb;
   529  int ret;
   530  int height;
   531  struct amdgpu_device *adev = dev->dev_private;
   532  int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
   533  int pitch = amdgpu_align_pitch(adev, mode_cmd->width, cpp, 
false);
   534  
   535  if (mode_cmd->pitches[0] != pitch) {
   536  dev_err(>pdev->dev, "Invalid pitch: expecting %d 
but got %d\n",
   537  pitch, mode_cmd->pitches[0]);
   538  return ERR_PTR(-EINVAL);
   539  }
   540  
   541  obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
   542  if (obj ==  NULL) {
   543  dev_err(>pdev->dev, "No GEM object associated to 
handle 0x%08X, "
   544  "can't create framebuffer\n", 
mode_cmd->handles[0]);
   545  return ERR_PTR(-ENOENT);
   546  }
   547  
   548  /* Handle is imported dma-buf, so cannot be migrated to VRAM 
for scanout */
   549  if (obj->import_attach) {
   550  DRM_DEBUG_KMS("Cannot create framebuffer from imported 
dma_buf\n");
   551  return ERR_PTR(-EINVAL);
   552  }
   553  
   554  height = ALIGN(mode_cmd->height, 8);
   555  if (obj->size < pitch * height) {
 > 556  dev_err(>pdev->dev, "Invalid GEM size: expecting 
 > %d but got %d\n",
   557  pitch * height, obj->size);
   558  return ERR_PTR(-EINVAL);
   559  }
   560  
   561  amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL);
   562  if (amdgpu_fb == NULL) {
   563  drm_gem_object_put_unlocked(obj);
   564  return ERR_PTR(-ENOMEM);
   565  }
   566  
   567  ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, mode_cmd, 
obj);
   568  if (ret) {
   569  kfree(amdgpu_fb);
   570  drm_gem_object_put_unlocked(obj);
   571  return ERR_PTR(ret);
   572  }
   573  
   574  return _fb->base;
   575  }
   576  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 3/3] drm/amd: validate user GEM object size

2018-12-21 Thread kbuild test robot
Hi Yu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.20-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Yu-Zhao/drm-amd-fix-race-in-page-flip-job/20181222-072937
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/cdev.h:8,
from include/drm/drmP.h:36,
from drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:26:
   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c: In function 
'amdgpu_display_user_framebuffer_create':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:556:28: warning: format '%d' 
>> expects argument of type 'int', but argument 4 has type 'size_t' {aka 'long 
>> unsigned int'} [-Wformat=]
  dev_err(>pdev->dev, "Invalid GEM size: expecting %d but got %d\n",
   ^
   include/linux/device.h:1370:22: note: in definition of macro 'dev_fmt'
#define dev_fmt(fmt) fmt
 ^~~
   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:556:3: note: in expansion of 
macro 'dev_err'
  dev_err(>pdev->dev, "Invalid GEM size: expecting %d but got %d\n",
  ^~~

vim +556 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c

   521  
   522  struct drm_framebuffer *
   523  amdgpu_display_user_framebuffer_create(struct drm_device *dev,
   524 struct drm_file *file_priv,
   525 const struct drm_mode_fb_cmd2 
*mode_cmd)
   526  {
   527  struct drm_gem_object *obj;
   528  struct amdgpu_framebuffer *amdgpu_fb;
   529  int ret;
   530  int height;
   531  struct amdgpu_device *adev = dev->dev_private;
   532  int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
   533  int pitch = amdgpu_align_pitch(adev, mode_cmd->width, cpp, 
false);
   534  
   535  if (mode_cmd->pitches[0] != pitch) {
   536  dev_err(>pdev->dev, "Invalid pitch: expecting %d 
but got %d\n",
   537  pitch, mode_cmd->pitches[0]);
   538  return ERR_PTR(-EINVAL);
   539  }
   540  
   541  obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
   542  if (obj ==  NULL) {
   543  dev_err(>pdev->dev, "No GEM object associated to 
handle 0x%08X, "
   544  "can't create framebuffer\n", 
mode_cmd->handles[0]);
   545  return ERR_PTR(-ENOENT);
   546  }
   547  
   548  /* Handle is imported dma-buf, so cannot be migrated to VRAM 
for scanout */
   549  if (obj->import_attach) {
   550  DRM_DEBUG_KMS("Cannot create framebuffer from imported 
dma_buf\n");
   551  return ERR_PTR(-EINVAL);
   552  }
   553  
   554  height = ALIGN(mode_cmd->height, 8);
   555  if (obj->size < pitch * height) {
 > 556  dev_err(>pdev->dev, "Invalid GEM size: expecting 
 > %d but got %d\n",
   557  pitch * height, obj->size);
   558  return ERR_PTR(-EINVAL);
   559  }
   560  
   561  amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL);
   562  if (amdgpu_fb == NULL) {
   563  drm_gem_object_put_unlocked(obj);
   564  return ERR_PTR(-ENOMEM);
   565  }
   566  
   567  ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, mode_cmd, 
obj);
   568  if (ret) {
   569  kfree(amdgpu_fb);
   570  drm_gem_object_put_unlocked(obj);
   571  return ERR_PTR(ret);
   572  }
   573  
   574  return _fb->base;
   575  }
   576  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 3/3] drm/amd: validate user GEM object size

2018-12-21 Thread Michel Dänzer
On 2018-12-21 4:10 a.m., Yu Zhao wrote:
> When creating frame buffer, userspace may request to attach to a
> previously allocated GEM object that is smaller than what GPU
> requires. Validation must be done to prevent out-of-bound DMA,
> which could not only corrupt memory but also reveal sensitive data.
> 
> This fix is not done in a common code path because individual
> driver might have different requirement.
> 
> Signed-off-by: Yu Zhao 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 755daa332f8a..bb48b016cc68 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -527,6 +527,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
> *dev,
>   struct drm_gem_object *obj;
>   struct amdgpu_framebuffer *amdgpu_fb;
>   int ret;
> + int height;
>   struct amdgpu_device *adev = dev->dev_private;
>   int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
>   int pitch = amdgpu_align_pitch(adev, mode_cmd->width, cpp, false);
> @@ -550,6 +551,13 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
> *dev,
>   return ERR_PTR(-EINVAL);
>   }
>  
> + height = ALIGN(mode_cmd->height, 8);
> + if (obj->size < pitch * height) {
> + dev_err(>pdev->dev, "Invalid GEM size: expecting %d but 
> got %d\n",
> + pitch * height, obj->size);

Same comment as patch 2, and maybe write "expecting >= %d but got %d"
for clarity.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx