All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix out of bounds writes in amdgpu_dm_debugfs
@ 2021-10-27 14:26 Harry Wentland
  2021-10-27 14:26 ` [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user Harry Wentland
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Harry Wentland @ 2021-10-27 14:26 UTC (permalink / raw)
  To: amd-gfx, sunpeng.li, alexander.deucher, christian.koenig,
	rodrigo.siqueira, patrik.r.jakobsson
  Cc: Harry Wentland

Fixes for out of bounds writes in amdgpu_dm_debugfs as well
as a fix to disallow partial copy_from_user so we don't
end up inadvertently reading corrupted buffers if a caller
fails to initialize the buffer to zero.

Harry Wentland (2):
  drm/amd/display: Don't allow partial copy_from_user
  drm/amd/display: Fix dp_max_bpc out of bounds write

Patrik Jakobsson (1):
  drm/amdgpu: Fix even more out of bound writes from debugfs

 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 28 +++++++++----------
 1 file changed, 13 insertions(+), 15 deletions(-)

--
2.33.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user
  2021-10-27 14:26 [PATCH 0/3] Fix out of bounds writes in amdgpu_dm_debugfs Harry Wentland
@ 2021-10-27 14:26 ` Harry Wentland
  2021-11-03 14:49   ` Rodrigo Siqueira Jordao
  2021-10-27 14:26 ` [PATCH 2/3] drm/amdgpu: Fix even more out of bound writes from debugfs Harry Wentland
  2021-10-27 14:26 ` [PATCH 3/3] drm/amd/display: Fix dp_max_bpc out of bounds write Harry Wentland
  2 siblings, 1 reply; 5+ messages in thread
From: Harry Wentland @ 2021-10-27 14:26 UTC (permalink / raw)
  To: amd-gfx, sunpeng.li, alexander.deucher, christian.koenig,
	rodrigo.siqueira, patrik.r.jakobsson
  Cc: Harry Wentland

There is no reason to allow for partial buffers from
userspace in our debugfs. In this particular case
callers will zero out the wr_buf but if callers in
the future don't do that we might be looking at
corrupt data.

Linus puts it better than I can in
https://lkml.org/lkml/2021/10/26/993

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c  | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 1a68a674913c..b30307ccff12 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -78,12 +78,10 @@ static int parse_write_buffer_into_params(char *wr_buf, uint32_t wr_buf_size,
 
 	wr_buf_ptr = wr_buf;
 
-	r = copy_from_user(wr_buf_ptr, buf, wr_buf_size);
-
-		/* r is bytes not be copied */
-	if (r >= wr_buf_size) {
-		DRM_DEBUG_DRIVER("user data not be read\n");
-		return -EINVAL;
+	/* r is bytes not be copied */
+	if (copy_from_user(wr_buf_ptr, buf, wr_buf_size)) {
+		DRM_DEBUG_DRIVER("user data could not be read successfully\n");
+		return -EFAULT;
 	}
 
 	/* check number of parameters. isspace could not differ space and \n */
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] drm/amdgpu: Fix even more out of bound writes from debugfs
  2021-10-27 14:26 [PATCH 0/3] Fix out of bounds writes in amdgpu_dm_debugfs Harry Wentland
  2021-10-27 14:26 ` [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user Harry Wentland
@ 2021-10-27 14:26 ` Harry Wentland
  2021-10-27 14:26 ` [PATCH 3/3] drm/amd/display: Fix dp_max_bpc out of bounds write Harry Wentland
  2 siblings, 0 replies; 5+ messages in thread
From: Harry Wentland @ 2021-10-27 14:26 UTC (permalink / raw)
  To: amd-gfx, sunpeng.li, alexander.deucher, christian.koenig,
	rodrigo.siqueira, patrik.r.jakobsson
  Cc: Patrik Jakobsson, Harry Wentland

From: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

CVE-2021-42327 was fixed by:

commit f23750b5b3d98653b31d4469592935ef6364ad67
Author: Thelford Williams <tdwilliamsiv@gmail.com>
Date:   Wed Oct 13 16:04:13 2021 -0400

    drm/amdgpu: fix out of bounds write

but amdgpu_dm_debugfs.c contains more of the same issue so fix the
remaining ones.

Fixes: 918698d5c2b5 ("drm/amd/display: Return the number of bytes parsed than allocated")
Signed-off-by: Patrik Jakobsson <pjakobsson@suse.de>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry
---
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c    | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index b30307ccff12..be8ff7a05030 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -489,7 +489,7 @@ static ssize_t dp_phy_settings_write(struct file *f, const char __user *buf,
 	if (!wr_buf)
 		return -ENOSPC;
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					   (long *)param, buf,
 					   max_param_num,
 					   &param_nums)) {
@@ -641,7 +641,7 @@ static ssize_t dp_phy_test_pattern_debugfs_write(struct file *f, const char __us
 	if (!wr_buf)
 		return -ENOSPC;
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					   (long *)param, buf,
 					   max_param_num,
 					   &param_nums)) {
@@ -916,7 +916,7 @@ static ssize_t dp_dsc_passthrough_set(struct file *f, const char __user *buf,
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					   &param, buf,
 					   max_param_num,
 					   &param_nums)) {
@@ -1213,7 +1213,7 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 						(long *)param, buf,
 						max_param_num,
 						&param_nums)) {
@@ -1398,7 +1398,7 @@ static ssize_t dp_dsc_clock_en_write(struct file *f, const char __user *buf,
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					    (long *)param, buf,
 					    max_param_num,
 					    &param_nums)) {
@@ -1583,7 +1583,7 @@ static ssize_t dp_dsc_slice_width_write(struct file *f, const char __user *buf,
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					    (long *)param, buf,
 					    max_param_num,
 					    &param_nums)) {
@@ -1768,7 +1768,7 @@ static ssize_t dp_dsc_slice_height_write(struct file *f, const char __user *buf,
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					    (long *)param, buf,
 					    max_param_num,
 					    &param_nums)) {
@@ -1946,7 +1946,7 @@ static ssize_t dp_dsc_bits_per_pixel_write(struct file *f, const char __user *bu
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					    (long *)param, buf,
 					    max_param_num,
 					    &param_nums)) {
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] drm/amd/display: Fix dp_max_bpc out of bounds write
  2021-10-27 14:26 [PATCH 0/3] Fix out of bounds writes in amdgpu_dm_debugfs Harry Wentland
  2021-10-27 14:26 ` [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user Harry Wentland
  2021-10-27 14:26 ` [PATCH 2/3] drm/amdgpu: Fix even more out of bound writes from debugfs Harry Wentland
@ 2021-10-27 14:26 ` Harry Wentland
  2 siblings, 0 replies; 5+ messages in thread
From: Harry Wentland @ 2021-10-27 14:26 UTC (permalink / raw)
  To: amd-gfx, sunpeng.li, alexander.deucher, christian.koenig,
	rodrigo.siqueira, patrik.r.jakobsson
  Cc: Harry Wentland

Fixes one more possible out of bounds write in
amdgpu_dm_debugfs.c.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index be8ff7a05030..9d43ecb1f692 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2384,7 +2384,7 @@ static ssize_t dp_max_bpc_write(struct file *f, const char __user *buf,
 		return -ENOSPC;
 	}
 
-	if (parse_write_buffer_into_params(wr_buf, size,
+	if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
 					   (long *)param, buf,
 					   max_param_num,
 					   &param_nums)) {
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user
  2021-10-27 14:26 ` [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user Harry Wentland
@ 2021-11-03 14:49   ` Rodrigo Siqueira Jordao
  0 siblings, 0 replies; 5+ messages in thread
From: Rodrigo Siqueira Jordao @ 2021-11-03 14:49 UTC (permalink / raw)
  To: Harry Wentland, amd-gfx, sunpeng.li, alexander.deucher,
	christian.koenig, rodrigo.siqueira, patrik.r.jakobsson

Hi Harry,

lgtm.
Btw, it looks like that the other patches from this series are already 
applied to amd-staging-drm-next.

Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>


On 2021-10-27 10:26 a.m., Harry Wentland wrote:
> There is no reason to allow for partial buffers from
> userspace in our debugfs. In this particular case
> callers will zero out the wr_buf but if callers in
> the future don't do that we might be looking at
> corrupt data.
> 
> Linus puts it better than I can in
> https://lkml.org/lkml/2021/10/26/993
> 
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c  | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 1a68a674913c..b30307ccff12 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -78,12 +78,10 @@ static int parse_write_buffer_into_params(char *wr_buf, uint32_t wr_buf_size,
>   
>   	wr_buf_ptr = wr_buf;
>   
> -	r = copy_from_user(wr_buf_ptr, buf, wr_buf_size);
> -
> -		/* r is bytes not be copied */
> -	if (r >= wr_buf_size) {
> -		DRM_DEBUG_DRIVER("user data not be read\n");
> -		return -EINVAL;
> +	/* r is bytes not be copied */
> +	if (copy_from_user(wr_buf_ptr, buf, wr_buf_size)) {
> +		DRM_DEBUG_DRIVER("user data could not be read successfully\n");
> +		return -EFAULT;
>   	}
>   
>   	/* check number of parameters. isspace could not differ space and \n */
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-03 14:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-27 14:26 [PATCH 0/3] Fix out of bounds writes in amdgpu_dm_debugfs Harry Wentland
2021-10-27 14:26 ` [PATCH 1/3] drm/amd/display: Don't allow partial copy_from_user Harry Wentland
2021-11-03 14:49   ` Rodrigo Siqueira Jordao
2021-10-27 14:26 ` [PATCH 2/3] drm/amdgpu: Fix even more out of bound writes from debugfs Harry Wentland
2021-10-27 14:26 ` [PATCH 3/3] drm/amd/display: Fix dp_max_bpc out of bounds write Harry Wentland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.