linux-kernel - Re: [PATCH] spi: spidev_test: Fix buffer overflow in unescape()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 9 Sep 2016 09:00:39 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Geert Uytterhoeven <geert+renesas@...der.be>
Cc:     Mark Brown <broonie@...nel.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        Adrian Remonda <adrianremonda@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH] spi: spidev_test: Fix buffer overflow in unescape()

On Thu, Sep 8, 2016 at 8:05 PM, Geert Uytterhoeven
<geert+renesas@...der.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@...der.be>
> Fixes: 30061915be6e3a2c ("spi: spidev_test: Added input buffer from the terminal")
> Cc: <stable@...r.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@...ux-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