Message ID | 20181221031053.240161-3-yuzhao@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/3] drm/amd: fix race in page flip job | expand |
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 <yuzhao@google.com> > --- > 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(&dev->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.
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(&dev->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(&dev->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(&dev->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(&dev->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(&dev->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 &amdgpu_fb->base; 575 } 576 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
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(&dev->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(&dev->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(&dev->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(&dev->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(&dev->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 &amdgpu_fb->base; 575 } 576 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
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(&dev->pdev->dev, "Invalid GEM size: expecting %d but got %d\n", + pitch * height, obj->size); + return ERR_PTR(-EINVAL); + } + amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL); if (amdgpu_fb == NULL) { drm_gem_object_put_unlocked(obj);
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 <yuzhao@google.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)