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.
Cc: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 686a26de50f9..883a4df2386d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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) {
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.
Cc: stable@vger.kernel.org # v4.2+ 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 883a4df2386d..a58fb8e021c6 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->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);
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.
Cc: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 686a26de50f9..af0626a2b528 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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) {
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.
Cc: stable@vger.kernel.org # v4.2+ 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 af0626a2b528..9aa23cb20873 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) { + 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);
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: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 15ce7e681d67..16af80ccd0a0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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) {
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.
Cc: stable@vger.kernel.org # v4.2+ 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 16af80ccd0a0..61c075a78ee1 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 = 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);
On 2018-12-30 2:00 a.m., Yu Zhao wrote:
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: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 15ce7e681d67..16af80ccd0a0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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);- }
This should possibly be tested in drm_internal_framebuffer_create instead.
- 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.
On Thu, Jan 03, 2019 at 05:33:19PM +0100, Michel Dänzer wrote:
On 2018-12-30 2:00 a.m., Yu Zhao wrote:
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: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 15ce7e681d67..16af80ccd0a0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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);- }
This should possibly be tested in drm_internal_framebuffer_create instead.
It seems to me drm_format_info_min_pitch() in framebuffer_check() already does it. We could just remove the check here?
On 2019-01-07 5:00 a.m., Yu Zhao wrote:
On Thu, Jan 03, 2019 at 05:33:19PM +0100, Michel Dänzer wrote:
On 2018-12-30 2:00 a.m., Yu Zhao wrote:
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: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 15ce7e681d67..16af80ccd0a0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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);- }
This should possibly be tested in drm_internal_framebuffer_create instead.
It seems to me drm_format_info_min_pitch() in framebuffer_check() already does it. We could just remove the check here?
Yeah, looks like that should be fine.
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: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 15ce7e681d67..de9f198d5371 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -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) {
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.
Cc: stable@vger.kernel.org # v4.2+ 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 de9f198d5371..32b7648b7ef4 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 = 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);
On 2019-01-07 11:51 p.m., Yu Zhao wrote:
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: stable@vger.kernel.org # v4.2+ Signed-off-by: Yu Zhao yuzhao@google.com
Both patches applied to amd-staging-drm-next (should land in 5.0), thanks!
linux-stable-mirror@lists.linaro.org
-
Michel Dänzer -
Yu Zhao