drm/amd: fix race in page flip job
Subsystems (3)
PATCH 1/3 drm/amd: fix race in page flip job
Fix race between page flip job submission and completion. We invoke page_flip callback to submit page flip job to GPU first and then set pflip_status. If GPU fires page flip done irq in between, its handler won't see the correct pflip_status thus will refuse to notify the job completion. The job will eventually times out.
Reverse the order of calling page_flip and setting pflip_status to fix the race.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +2 -2
@@ -105,11 +105,11 @@ static void amdgpu_display_flip_work_func(struct work_struct *__work)/* We borrow the event spin lock for protecting flip_status */ spin_lock_irqsave (&crtc->dev->event_lock, flags); +/* Set the flip status */ + amdgpu_crtc->pflip_status =AMDGPU_FLIP_SUBMITTED ;/* Do the flip (mmio) */ adev->mode_info.funcs->page_flip (adev, work->crtc_id, work->base, work->async); -/* Set the flip status */ - amdgpu_crtc->pflip_status =AMDGPU_FLIP_SUBMITTED ;spin_unlock_irqrestore (&crtc->dev->event_lock, flags);
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH 2/3 drm/amd: validate user pitch alignment
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.
For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +9 -0
@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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); + +if (mode_cmd->pitches[0 ] != pitch) { +dev_err (&dev->pdev->dev,"Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Signature · 1 line
2.20.1.415.g653613c723-goog
Re: [PATCH 2/3] drm/amd: validate user pitch alignment
Quoted 18 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +0
-0
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 536
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +7
-0@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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); + +if (mode_cmd->pitches[0 ] != pitch) { +dev_err (&dev->pdev->dev,"Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]);
This message shouldn't be printed by default, or buggy / malicious userspace can spam dmesg. I suggest using DRM_DEBUG_KMS or DRM_DEBUG_DRIVER.
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 541
Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +2
-0+return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Other than that, looks good to me.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm/amd: validate user pitch alignment
In reply to Yu ZhaoQuoted 18 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +0
-0
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 532
Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +3
-0@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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);
Also, this needs to use mode_cmd->pitches[0] instead of mode_cmd->width, otherwise it'll spuriously fail for larger but well-aligned pitches.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm/amd: validate user pitch alignment
Quoted 14 lines · Michel Dänzer
Quoted 13 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Quoted 14 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 532
Quoted 11 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +3
-0@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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);Also, this needs to use mode_cmd->pitches[0] instead of
mode_cmd->width, otherwise it'll spuriously fail for larger but well-aligned pitches.
Good point, thanks.
Re: [PATCH 2/3] drm/amd: validate user pitch alignment
In reply to Michel DänzerQuoted 14 lines · Michel Dänzer
Quoted 13 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Quoted 14 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 532
Quoted 11 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +3
-0@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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);Also, this needs to use mode_cmd->pitches[0] instead of
mode_cmd->width, otherwise it'll spuriously fail for larger but well-aligned pitches.
Actually mode_cmd->pitches[0] is aligned mode_cmd->width multiplied by cpp. So we can't just use mode_cmd->pitches[0]. And I'm not sure if the hardware works with larger alignment (it certainly ignores smaller alignment).
Re: [PATCH 2/3] drm/amd: validate user pitch alignment
Quoted 30 lines · Yu Zhao
Quoted 14 lines · Michel Dänzer
Quoted 13 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)Quoted 14 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 532
Quoted 11 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +3
-0@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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);Also, this needs to use mode_cmd->pitches[0] instead of
mode_cmd->width, otherwise it'll spuriously fail for larger but well-aligned pitches.
Quoted 2 lines
Actually mode_cmd->pitches[0] is aligned
mode_cmd->widthmultiplied by cpp. So we can't just use mode_cmd->pitches[0].
Use mode_cmd->pitches[0] / cpp then?
Quoted 1 line
And I'm not sure if the hardware works with larger alignment
It does.
Quoted 1 line
(it certainly ignores smaller alignment).
Yeah, pitch must be >= width. Maybe this patch could check that as well.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 1/2 drm/amd: validate user pitch alignment
In reply to Yu ZhaoUserspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.
For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +9 -0
@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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->pitches[0 ], cpp, false); + +if (mode_cmd->pitches[0 ] != pitch) { +DRM_DEBUG_KMS ("Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH v2 2/2 drm/amd: validate user GEM object size
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.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +8 -0
@@ -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->pitches[0 ], 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) { +DRM_DEBUG_KMS ("Invalid GEM size: expecting >= %d but got %ld\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);
Signature · 1 line
2.20.1.415.g653613c723-goog
kbuild test robot
build botRe: [PATCH v2 2/2] drm/amd: validate user GEM object size
Thank you for the patch! Perhaps something to improve:
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-validate-user-pitch-alignment/20181222-153630
config: i386-allmodconfig (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=i386 All warnings (new ones prefixed by >>):
In file included from include/drm/drm_mm.h:49:0,
from include/drm/drmP.h:73,
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:17: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'size_t {aka unsigned int}' [-Wformat=]
DRM_DEBUG_KMS("Invalid GEM size: expecting >= %d but got %ld\n",
^
pitch * height, obj->size);
~~~~~~
include/drm/drm_print.h:362:22: note: in definition of macro 'DRM_DEBUG_KMS'
drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
^~~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->pitches[0], cpp, false);
534
535 if (mode_cmd->pitches[0] != pitch) {
536 DRM_DEBUG_KMS("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 DRM_DEBUG_KMS("Invalid GEM size: expecting >= %d but got %ld\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
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
kbuild test robot
build botRe: [PATCH v2 2/2] drm/amd: validate user GEM object size
In reply to Yu ZhaoThank you for the patch! Perhaps something to improve:
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-validate-user-pitch-alignment/20181222-153630
config: i386-randconfig-h1-12231406 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 reproduce:
# save the attached .config to linux build tree
make ARCH=i386 All warnings (new ones prefixed by >>):
drivers/gpu//drm/amd/amdgpu/amdgpu_display.c: In function 'amdgpu_display_user_framebuffer_create':
>> drivers/gpu//drm/amd/amdgpu/amdgpu_display.c:556:3: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'size_t' [-Wformat=]
DRM_DEBUG_KMS("Invalid GEM size: expecting >= %d but got %ld\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->pitches[0], cpp, false);
534
535 if (mode_cmd->pitches[0] != pitch) {
536 DRM_DEBUG_KMS("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 DRM_DEBUG_KMS("Invalid GEM size: expecting >= %d but got %ld\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
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH v3 1/2 drm/amd: validate user pitch alignment
In reply to Yu ZhaoUserspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.
For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +9 -0
@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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->pitches[0 ], cpp, false); + +if (mode_cmd->pitches[0 ] != pitch) { +DRM_DEBUG_KMS ("Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH v3 2/2 drm/amd: validate user GEM object size
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.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +8 -0
@@ -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->pitches[0 ], 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) { +DRM_DEBUG_KMS ("Invalid GEM size: expecting >= %d but got %zu\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);
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH v4 1/2 drm/amd: validate user pitch alignment
In reply to Yu ZhaoUserspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.
For GPU that does frame buffer compression, DMA writing out of bound memory will cause memory corruption.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +9 -0
@@ -527,6 +527,15 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +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); + +if (mode_cmd->pitches[0 ] != pitch) { +DRM_DEBUG_KMS ("Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH v4 2/2 drm/amd: validate user GEM object size
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.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +8 -0
@@ -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) { +DRM_DEBUG_KMS ("Invalid GEM size: expecting >= %d but got %zu\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);
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH v5 1/2 drm/amd: validate user pitch alignment
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +16 -0
@@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +struct amdgpu_device *adev = dev->dev_private; +int cpp =drm_format_plane_cpp (mode_cmd->pixel_format,0 ); +int pitch = mode_cmd->pitches[0 ] / cpp; + +if (pitch < mode_cmd->width) { +DRM_DEBUG_KMS ("Expecting pitch(%d)/cpp(%d) >= width(%d)\n" , + mode_cmd->pitches[0 ], cpp, mode_cmd->width); +return ERR_PTR (-EINVAL ); + } + + pitch =amdgpu_align_pitch (adev, pitch, cpp, false); +if (mode_cmd->pitches[0 ] != pitch) { +DRM_DEBUG_KMS ("Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Signature · 1 line
2.20.1.415.g653613c723-goog
PATCH v5 2/2 drm/amd: validate user GEM object size
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, otherwise it could be exploited to reveal sensitive data.
This fix is not done in a common code path because individual driver might have different requirement.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +8 -0
@@ -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 = mode_cmd->pitches[0 ] / cpp;
@@ -557,6 +558,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) { +DRM_DEBUG_KMS ("Invalid GEM size: expecting >= %d but got %zu\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);
Signature · 1 line
2.20.1.415.g653613c723-goog
Re: [PATCH v5 1/2] drm/amd: validate user pitch alignment
In reply to Yu ZhaoQuoted 18 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.Cc [email protected] # v4.2+Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +0
-0@@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 538
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +9
-0int ret; +struct amdgpu_device *adev = dev->dev_private; +int cpp =drm_format_plane_cpp (mode_cmd->pixel_format,0 ); +int pitch = mode_cmd->pitches[0 ] / cpp; + +if (pitch < mode_cmd->width) { +DRM_DEBUG_KMS ("Expecting pitch(%d)/cpp(%d) >= width(%d)\n" , + mode_cmd->pitches[0 ], cpp, mode_cmd->width); +return ERR_PTR (-EINVAL ); + }
This should possibly be tested in drm_internal_framebuffer_create instead.
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 548
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +6
-0+ pitch =amdgpu_align_pitch (adev, pitch, cpp, false); +if (mode_cmd->pitches[0 ] != pitch) { +DRM_DEBUG_KMS ("Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Apart from the above, the v5 series looks good to me.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/2] drm/amd: validate user pitch alignment
Quoted 23 lines · Michel Dänzer
Quoted 22 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.Cc [email protected] # v4.2+Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +3
-0@@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +struct amdgpu_device *adev = dev->dev_private; +int cpp =drm_format_plane_cpp (mode_cmd->pixel_format,0 ); +int pitch = mode_cmd->pitches[0 ] / cpp;
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 538
Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +6
-0+ +if (pitch < mode_cmd->width) { +DRM_DEBUG_KMS ("Expecting pitch(%d)/cpp(%d) >= width(%d)\n" , + mode_cmd->pitches[0 ], cpp, mode_cmd->width); +return ERR_PTR (-EINVAL ); + }This should possibly be tested in
drm_internal_framebuffer_createinstead.
It seems to me drm_format_info_min_pitch() in framebuffer_check() already does it. We could just remove the check here?
Re: [PATCH v5 1/2] drm/amd: validate user pitch alignment
Quoted 33 lines · Yu Zhao
Quoted 23 lines · Michel Dänzer
Quoted 22 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.Cc [email protected] # v4.2+Signed-off-by Yu Zhao <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +3
-0@@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +struct amdgpu_device *adev = dev->dev_private; +int cpp =drm_format_plane_cpp (mode_cmd->pixel_format,0 ); +int pitch = mode_cmd->pitches[0 ] / cpp;Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 538
Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +6
-0+ +if (pitch < mode_cmd->width) { +DRM_DEBUG_KMS ("Expecting pitch(%d)/cpp(%d) >= width(%d)\n" , + mode_cmd->pitches[0 ], cpp, mode_cmd->width); +return ERR_PTR (-EINVAL ); + }This should possibly be tested in
drm_internal_framebuffer_createinstead.
Quoted 2 lines
It seems to me
drm_format_info_min_pitch()inframebuffer_check()already does it. We could just remove the check here?
Yeah, looks like that should be fine.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH v6 1/2 drm/amd: validate user pitch alignment
In reply to Yu ZhaoUserspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +10 -0
@@ -527,6 +527,16 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,struct drm_gem_object *obj;struct amdgpu_framebuffer *amdgpu_fb;int ret; +struct amdgpu_device *adev = dev->dev_private; +int cpp =drm_format_plane_cpp (mode_cmd->pixel_format,0 ); +int pitch = mode_cmd->pitches[0 ] / cpp; + + pitch =amdgpu_align_pitch (adev, pitch, cpp, false); +if (mode_cmd->pitches[0 ] != pitch) { +DRM_DEBUG_KMS ("Invalid pitch: expecting %d but got %d\n" , + pitch, mode_cmd->pitches[0 ]); +return ERR_PTR (-EINVAL ); + } obj =drm_gem_object_lookup (file_priv, mode_cmd->handles[0 ]);if (obj ==NULL ) {
Signature · 1 line
2.20.1.97.g81188d93c3-goog
PATCH v6 2/2 drm/amd: validate user GEM object size
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, otherwise it could be exploited to reveal sensitive data.
This fix is not done in a common code path because individual driver might have different requirement.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +8 -0
@@ -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 = mode_cmd->pitches[0 ] / cpp;
@@ -551,6 +552,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) { +DRM_DEBUG_KMS ("Invalid GEM size: expecting >= %d but got %zu\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);
Signature · 1 line
2.20.1.97.g81188d93c3-goog
Re: [PATCH v6 1/2] drm/amd: validate user pitch alignment
In reply to Yu ZhaoQuoted 7 lines · Yu Zhao
Userspace may request pitch alignment that is not supported by GPU. Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU DMA will go out of bound.Cc [email protected] # v4.2+Signed-off-by Yu Zhao <[email protected]>
Both patches applied to amd-staging-drm-next (should land in 5.0), thanks!
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 3/3 drm/amd: validate user GEM object size
In reply to Yu ZhaoWhen 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.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +8 -0
@@ -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);
Signature · 1 line
2.20.1.415.g653613c723-goog
Re: [PATCH 3/3] drm/amd: validate user GEM object size
Quoted 26 lines · Yu Zhao
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 <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +1
-0@@ -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);
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c amdgpu_display_user_framebuffer_create(struct drm_device *dev, · line 557
Quoted 7 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +4
-0@@ -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.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
kbuild test robot
build botRe: [PATCH 3/3] drm/amd: validate user GEM object size
In reply to Yu ZhaoThank you for the patch! Perhaps something to improve:
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
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
kbuild test robot
build botRe: [PATCH 3/3] drm/amd: validate user GEM object size
In reply to Yu ZhaoThank you for the patch! Perhaps something to improve:
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
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Quoted 8 lines · Yu Zhao
Fix race between page flip job submission and completion. We invoke
page_flipcallback to submit page flip job to GPU first and then setpflip_status. If GPU fires page flip done irq in between, its handler won't see the correctpflip_statusthus will refuse to notify the job completion. The job will eventually times out.Reverse the order of calling
page_flipand settingpflip_statusto fix the race.
There is no race, amdgpu_crtc->pflip_status is protected by dev->event_lock.
Signature · 6 lines
Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel