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

[Masami Hiramatsu <mhiramat@kernel.org>]

On Fri, 26 Jul 2019 16:38:06 -0300
Arnaldo Carvalho de Melo <acme@kernel.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@quaco 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@quaco 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@quaco perf]$
> 
> 
> - Arnaldo
>  
> > Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.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@kernel.org>