All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: spidev_test: Fix buffer overflow in unescape()
@ 2016-09-08 18:05 ` Geert Uytterhoeven
  0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2016-09-08 18:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, Adrian Remonda, linux-kernel, Geert Uytterhoeven, stable

Sometimes spidev_test crashes with:

    *** Error in `spidev_test': munmap_chunk(): invalid pointer: 0x00022020 ***
    Aborted

or just

    Segmentation fault

This is due to transfer_escaped_string() miscalculating the required
size of the buffer by two bytes, causing a buffer overflow in unescape().

Move the misplaced closing parenthesis to fix this.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Fixes: 30061915be6e3a2c ("spi: spidev_test: Added input buffer from the terminal")
Cc: <stable@vger.kernel.org> # v4.5+
---
The bug is present in all kernels since v4.1, but in v4.5 the code was
changed, and the source file was moved.

The fix for older kernels is straight-forward, there's only a single
strlen() call in Documentation/spi/spidev_test.c:

	-	size = strlen(input_tx+1);
	+	size = strlen(input_tx)+1;

If you want, I can send a patch against v4.4 (for v4.1..v4.4) later.
---
 tools/spi/spidev_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
index 1eaa4de6605bd935..ffa9908f7eb6670e 100644
--- a/tools/spi/spidev_test.c
+++ b/tools/spi/spidev_test.c
@@ -285,7 +285,7 @@ static void parse_opts(int argc, char *argv[])
 
 static void transfer_escaped_string(int fd, char *str)
 {
-	size_t size = strlen(str + 1);
+	size_t size = strlen(str) + 1;
 	uint8_t *tx;
 	uint8_t *rx;
 
-- 
1.9.1

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

* [PATCH] spi: spidev_test: Fix buffer overflow in unescape()
@ 2016-09-08 18:05 ` Geert Uytterhoeven
  0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2016-09-08 18:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Adrian Remonda,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven,
	stable-u79uwXL29TY76Z2rM5mHXA

Sometimes spidev_test crashes with:

    *** Error in `spidev_test': munmap_chunk(): invalid pointer: 0x00022020 ***
    Aborted

or just

    Segmentation fault

This is due to transfer_escaped_string() miscalculating the required
size of the buffer by two bytes, causing a buffer overflow in unescape().

Move the misplaced closing parenthesis to fix this.

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Fixes: 30061915be6e3a2c ("spi: spidev_test: Added input buffer from the terminal")
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.5+
---
The bug is present in all kernels since v4.1, but in v4.5 the code was
changed, and the source file was moved.

The fix for older kernels is straight-forward, there's only a single
strlen() call in Documentation/spi/spidev_test.c:

	-	size = strlen(input_tx+1);
	+	size = strlen(input_tx)+1;

If you want, I can send a patch against v4.4 (for v4.1..v4.4) later.
---
 tools/spi/spidev_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
index 1eaa4de6605bd935..ffa9908f7eb6670e 100644
--- a/tools/spi/spidev_test.c
+++ b/tools/spi/spidev_test.c
@@ -285,7 +285,7 @@ static void parse_opts(int argc, char *argv[])
 
 static void transfer_escaped_string(int fd, char *str)
 {
-	size_t size = strlen(str + 1);
+	size_t size = strlen(str) + 1;
 	uint8_t *tx;
 	uint8_t *rx;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] spi: spidev_test: Fix buffer overflow in unescape()
@ 2016-09-09  7:00   ` Geert Uytterhoeven
  0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2016-09-09  7:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, linux-spi, Adrian Remonda, linux-kernel, stable

On Thu, Sep 8, 2016 at 8:05 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> Sometimes spidev_test crashes with:
>
>     *** Error in `spidev_test': munmap_chunk(): invalid pointer: 0x00022020 ***
>     Aborted
>
> or just
>
>     Segmentation fault
>
> This is due to transfer_escaped_string() miscalculating the required
> size of the buffer by two bytes, causing a buffer overflow in unescape().
>
> Move the misplaced closing parenthesis to fix this.

After one more night of sleep, I realized it's an off-by-one, not off-by-two
bug (unescape() doesn't copy the zero-terminator of the source string).

Will send v2 in a moment...

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Fixes: 30061915be6e3a2c ("spi: spidev_test: Added input buffer from the terminal")
> Cc: <stable@vger.kernel.org> # v4.5+
> ---
> The bug is present in all kernels since v4.1, but in v4.5 the code was
> changed, and the source file was moved.
>
> The fix for older kernels is straight-forward, there's only a single
> strlen() call in Documentation/spi/spidev_test.c:
>
>         -       size = strlen(input_tx+1);
>         +       size = strlen(input_tx)+1;
>
> If you want, I can send a patch against v4.4 (for v4.1..v4.4) later.
> ---
>  tools/spi/spidev_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
> index 1eaa4de6605bd935..ffa9908f7eb6670e 100644
> --- a/tools/spi/spidev_test.c
> +++ b/tools/spi/spidev_test.c
> @@ -285,7 +285,7 @@ static void parse_opts(int argc, char *argv[])
>
>  static void transfer_escaped_string(int fd, char *str)
>  {
> -       size_t size = strlen(str + 1);
> +       size_t size = strlen(str) + 1;
>         uint8_t *tx;
>         uint8_t *rx;
>
> --
> 1.9.1

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH] spi: spidev_test: Fix buffer overflow in unescape()
@ 2016-09-09  7:00   ` Geert Uytterhoeven
  0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2016-09-09  7:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, linux-spi, Adrian Remonda,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, stable

On Thu, Sep 8, 2016 at 8:05 PM, Geert Uytterhoeven
<geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> wrote:
> Sometimes spidev_test crashes with:
>
>     *** Error in `spidev_test': munmap_chunk(): invalid pointer: 0x00022020 ***
>     Aborted
>
> or just
>
>     Segmentation fault
>
> This is due to transfer_escaped_string() miscalculating the required
> size of the buffer by two bytes, causing a buffer overflow in unescape().
>
> Move the misplaced closing parenthesis to fix this.

After one more night of sleep, I realized it's an off-by-one, not off-by-two
bug (unescape() doesn't copy the zero-terminator of the source string).

Will send v2 in a moment...

> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> Fixes: 30061915be6e3a2c ("spi: spidev_test: Added input buffer from the terminal")
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.5+
> ---
> The bug is present in all kernels since v4.1, but in v4.5 the code was
> changed, and the source file was moved.
>
> The fix for older kernels is straight-forward, there's only a single
> strlen() call in Documentation/spi/spidev_test.c:
>
>         -       size = strlen(input_tx+1);
>         +       size = strlen(input_tx)+1;
>
> If you want, I can send a patch against v4.4 (for v4.1..v4.4) later.
> ---
>  tools/spi/spidev_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
> index 1eaa4de6605bd935..ffa9908f7eb6670e 100644
> --- a/tools/spi/spidev_test.c
> +++ b/tools/spi/spidev_test.c
> @@ -285,7 +285,7 @@ static void parse_opts(int argc, char *argv[])
>
>  static void transfer_escaped_string(int fd, char *str)
>  {
> -       size_t size = strlen(str + 1);
> +       size_t size = strlen(str) + 1;
>         uint8_t *tx;
>         uint8_t *rx;
>
> --
> 1.9.1

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2016-09-09  7:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-08 18:05 [PATCH] spi: spidev_test: Fix buffer overflow in unescape() Geert Uytterhoeven
2016-09-08 18:05 ` Geert Uytterhoeven
2016-09-09  7:00 ` Geert Uytterhoeven
2016-09-09  7:00   ` Geert Uytterhoeven

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.