[PATCH] drm: fix off-by-one in logger

[PATCH] drm: fix off-by-one in logger

[Norbert Manthey]

The current implementation will leak a byte to the log via memmove. The
specified 27 bytes are off-by-one, as the payload is 25 bytes, and the
termination character is only one byte large. To avoid this, factor out
the error message, and furthermore make the second parameter of the
append_entry function const.

Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")

The full trace is as follows:

In function ‘memmove’,
   from ‘append_entry’ at
        drivers/gpu/drm/amd/display/dc/basics/logger.c:257:2,
   from ‘dm_logger_append_va’ at
        drivers/gpu/drm/amd/display/dc/basics/logger.c:348:4
   detected read beyond size of object passed as 2nd parameter

Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Tony Cheng <tony.cheng@amd.com>
Cc: Yongqiang Sun <yongqiang.sun@amd.com>
Cc: Aric Cyr <Aric.Cyr@amd.com>
Cc: Colin Ian King <colin.king@canonical.com>
Cc: Corbin McElhanney <corbin.mcelhanney@amd.com>
Cc: Jordan Lazare <Jordan.Lazare@amd.com>
Cc: Dmytro Laktyushkin <Dmytro.Laktyushkin@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org

---
 drivers/gpu/drm/amd/display/dc/basics/logger.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/basics/logger.c b/drivers/gpu/drm/amd/display/dc/basics/logger.c
index 180a9d6..958070c 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/logger.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/logger.c
@@ -243,7 +243,7 @@ static void log_heading(struct log_entry *entry)
 
 static void append_entry(
 		struct log_entry *entry,
-		char *buffer,
+		const char *buffer,
 		uint32_t buf_size)
 {
 	if (!entry->buf ||
@@ -345,7 +345,9 @@ void dm_logger_append_va(
 		if (size < LOG_MAX_LINE_SIZE - 1) {
 			append_entry(entry, buffer, size);
 		} else {
-			append_entry(entry, "LOG_ERROR, line too long\n", 27);
+			static const char msg[] = "LOG_ERROR, line too long\n";
+
+			append_entry(entry, msg, sizeof(msg));
 		}
 	}
 }
-- 
2.7.4

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH] drm: fix off-by-one in logger

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2635 bytes --]



On Fri, 2018-02-16 at 10:43 +0100, Norbert Manthey wrote:
> The current implementation will leak a byte to the log via memmove. The
> specified 27 bytes are off-by-one, as the payload is 25 bytes, and the
> termination character is only one byte large. To avoid this, factor out
> the error message, and furthermore make the second parameter of the
> append_entry function const.
> 
> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
> 
> The full trace is as follows:
> 
> In function ‘memmove’,
>    from ‘append_entry’ at
>         drivers/gpu/drm/amd/display/dc/basics/logger.c:257:2,
>    from ‘dm_logger_append_va’ at
>         drivers/gpu/drm/amd/display/dc/basics/logger.c:348:4
>    detected read beyond size of object passed as 2nd parameter
> 
> Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

That same code exists in a different form in at least 4.15 so

Cc: stable@vger.kernel.org

> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Tony Cheng <tony.cheng@amd.com>
> Cc: Yongqiang Sun <yongqiang.sun@amd.com>
> Cc: Aric Cyr <Aric.Cyr@amd.com>
> Cc: Colin Ian King <colin.king@canonical.com>
> Cc: Corbin McElhanney <corbin.mcelhanney@amd.com>
> Cc: Jordan Lazare <Jordan.Lazare@amd.com>
> Cc: Dmytro Laktyushkin <Dmytro.Laktyushkin@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> 
> ---
>  drivers/gpu/drm/amd/display/dc/basics/logger.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/logger.c b/drivers/gpu/drm/amd/display/dc/basics/logger.c
> index 180a9d6..958070c 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/logger.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/logger.c
> @@ -243,7 +243,7 @@ static void log_heading(struct log_entry *entry)
>  
>  static void append_entry(
>  		struct log_entry *entry,
> -		char *buffer,
> +		const char *buffer,
>  		uint32_t buf_size)
>  {
>  	if (!entry->buf ||
> @@ -345,7 +345,9 @@ void dm_logger_append_va(
>  		if (size < LOG_MAX_LINE_SIZE - 1) {
>  			append_entry(entry, buffer, size);
>  		} else {
> -			append_entry(entry, "LOG_ERROR, line too long\n", 27);
> +			static const char msg[] = "LOG_ERROR, line too long\n";
> +
> +			append_entry(entry, msg, sizeof(msg));
>  		}
>  	}
>  }

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]