linux-kernel - Re: [PATCH 3/3] Fix insn.c misaligned address error

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] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 27 Jul 2019 18:46:38 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        Numfor Mbiziwo-Tiapo <nums@...gle.com>, peterz@...radead.org,
        mingo@...hat.com, alexander.shishkin@...ux.intel.com,
        jolsa@...hat.com, namhyung@...nel.org, songliubraving@...com,
        mbd@...com, linux-kernel@...r.kernel.org, irogers@...gle.com,
        eranian@...gle.com
Subject: Re: [PATCH 3/3] Fix insn.c misaligned address error

On Fri, 26 Jul 2019 16:38:06 -0300
Arnaldo Carvalho de Melo <acme@...nel.org> wrote:

> Em Wed, Jul 24, 2019 at 11:45:12AM -0700, Numfor Mbiziwo-Tiapo escreveu:
> > The ubsan (undefined behavior sanitizer) version of perf throws an
> > error on the 'x86 instruction decoder - new instructions' function
> > of perf test.
> > 
> > To reproduce this run:
> > make -C tools/perf USE_CLANG=1 EXTRA_CFLAGS="-fsanitize=undefined"
> > 
> > then run: tools/perf/perf test 62 -v
> > 
> > The error occurs in the __get_next macro (line 34) where an int is
> > read from a potentially unaligned address. Using memcpy instead of
> > assignment from an unaligned pointer.
> 
> Since this came from the kernel, don't we have to fix it there as well?
> Masami, Adrian?

I guess we don't need it, since x86 can access "unaligned address" and
x86 insn decoder in kernel runs only on x86. I'm not sure about perf's
that part. Maybe if we run it on other arch as cross-arch application,
it may cause unaligned pointer issue.

Thank you,

> 
> [acme@...co perf]$ find . -name insn.c
> ./arch/x86/lib/insn.c
> ./arch/arm/kernel/insn.c
> ./arch/arm64/kernel/insn.c
> ./tools/objtool/arch/x86/lib/insn.c
> ./tools/perf/util/intel-pt-decoder/insn.c
> [acme@...co perf]$ diff -u ./tools/perf/util/intel-pt-decoder/insn.c ./arch/x86/lib/insn.c
> --- ./tools/perf/util/intel-pt-decoder/insn.c	2019-07-06 16:59:05.734265998 -0300
> +++ ./arch/x86/lib/insn.c	2019-07-06 16:59:01.369202998 -0300
> @@ -10,8 +10,8 @@
>  #else
>  #include <string.h>
>  #endif
> -#include "inat.h"
> -#include "insn.h"
> +#include <asm/inat.h>
> +#include <asm/insn.h>
> 
>  /* Verify next sizeof(t) bytes can be on the same instruction */
>  #define validate_next(t, insn, n)	\
> [acme@...co perf]$
> 
> 
> - Arnaldo
>  
> > Signed-off-by: Numfor Mbiziwo-Tiapo <nums@...gle.com>
> > ---
> >  tools/perf/util/intel-pt-decoder/insn.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/intel-pt-decoder/insn.c b/tools/perf/util/intel-pt-decoder/insn.c
> > index ca983e2bea8b..de1944c60aa9 100644
> > --- a/tools/perf/util/intel-pt-decoder/insn.c
> > +++ b/tools/perf/util/intel-pt-decoder/insn.c
> > @@ -31,7 +31,8 @@
> >  	((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
> >  
> >  #define __get_next(t, insn)	\
> > -	({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
> > +	({ t r; memcpy(&r, insn->next_byte, sizeof(t)); \
> > +		insn->next_byte += sizeof(t); r; })
> >  
> >  #define __peek_nbyte_next(t, insn, n)	\
> >  	({ t r = *(t*)((insn)->next_byte + n); r; })
> > -- 
> > 2.22.0.657.g960e92d24f-goog
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@...nel.org>