Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1257892 > unrolled thread

[RFT, PATCH v1 1/1] hexdump: truncate output in case of overflow

Started byAndy Shevchenko <andriy.shevchenko@linux.intel.com>
First post2015-10-28 11:50 +0100
Last post2015-10-28 14:30 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFT, PATCH v1 1/1] hexdump: truncate output in case of overflow Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-10-28 11:50 +0100
    Re: [RFT, PATCH v1 1/1] hexdump: truncate output in case of overflow Aaro Koskinen <aaro.koskinen@nokia.com> - 2015-10-28 14:30 +0100

#1257892 — [RFT, PATCH v1 1/1] hexdump: truncate output in case of overflow

FromAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Date2015-10-28 11:50 +0100
Subject[RFT, PATCH v1 1/1] hexdump: truncate output in case of overflow
Message-ID<qowsN-1tK-5@gated-at.bofh.it>
There is a classical off-by-one error in case when we try to place, for
example, 1+1 bytes as hex in the buffer of size 6. The expected result is to
get an output truncated, but in the reality we get 6 bytes filed followed by
terminating NUL.

Change the logic how we fill the output in case of byte dumping into limited
space. This will follow the snprintf() behaviour by truncating output even on
half bytes.

Fixes: 114fc1afb2de (hexdump: make it return number of bytes placed in buffer)
Reported-by: Aaro Koskinen <aaro.koskinen@nokia.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Waiting for Aaro's Tested-by: tag, that's why RFT. Meanwhile I will update
test-hexdump to cover all corner case in overflow.

Linus, it would be nice to promote the fix when we get Aaro's confirmation.

 lib/hexdump.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/hexdump.c b/lib/hexdump.c
index 8d74c20..992457b 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -169,11 +169,15 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 		}
 	} else {
 		for (j = 0; j < len; j++) {
-			if (linebuflen < lx + 3)
+			if (linebuflen < lx + 2)
 				goto overflow2;
 			ch = ptr[j];
 			linebuf[lx++] = hex_asc_hi(ch);
+			if (linebuflen < lx + 2)
+				goto overflow2;
 			linebuf[lx++] = hex_asc_lo(ch);
+			if (linebuflen < lx + 2)
+				goto overflow2;
 			linebuf[lx++] = ' ';
 		}
 		if (j)
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1258020

FromAaro Koskinen <aaro.koskinen@nokia.com>
Date2015-10-28 14:30 +0100
Message-ID<qoyXD-38X-17@gated-at.bofh.it>
In reply to#1257892
Hi,

On Wed, Oct 28, 2015 at 12:48:19PM +0200, Andy Shevchenko wrote:
> There is a classical off-by-one error in case when we try to place, for
> example, 1+1 bytes as hex in the buffer of size 6. The expected result is to
> get an output truncated, but in the reality we get 6 bytes filed followed by
> terminating NUL.
> 
> Change the logic how we fill the output in case of byte dumping into limited
> space. This will follow the snprintf() behaviour by truncating output even on
> half bytes.
> 
> Fixes: 114fc1afb2de (hexdump: make it return number of bytes placed in buffer)
> Reported-by: Aaro Koskinen <aaro.koskinen@nokia.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks, this fixes the crash with kmemleak for me:

Tested-by: Aaro Koskinen <aaro.koskinen@nokia.com>

A.

> ---
> Waiting for Aaro's Tested-by: tag, that's why RFT. Meanwhile I will update
> test-hexdump to cover all corner case in overflow.
> 
> Linus, it would be nice to promote the fix when we get Aaro's confirmation.
> 
>  lib/hexdump.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 8d74c20..992457b 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -169,11 +169,15 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>  		}
>  	} else {
>  		for (j = 0; j < len; j++) {
> -			if (linebuflen < lx + 3)
> +			if (linebuflen < lx + 2)
>  				goto overflow2;
>  			ch = ptr[j];
>  			linebuf[lx++] = hex_asc_hi(ch);
> +			if (linebuflen < lx + 2)
> +				goto overflow2;
>  			linebuf[lx++] = hex_asc_lo(ch);
> +			if (linebuflen < lx + 2)
> +				goto overflow2;
>  			linebuf[lx++] = ' ';
>  		}
>  		if (j)
> -- 
> 2.6.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web