linux-kernel - Re: perf bpf: Reworked fix endianness problem when loading parameters in prologue

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

Message-ID: <e0061fb8-69c1-5719-9df4-f8affd16d192@huawei.com>
Date:   Mon, 14 Aug 2017 18:30:20 +0800
From:   "Wangnan (F)" <wangnan0@...wei.com>
To:     Thomas Richter <tmricht@...ux.vnet.ibm.com>, <acme@...nel.org>,
        <linux-perf-users@...r.kernel.org>
CC:     <brueckner@...ux.vnet.ibm.com>, <zvonko.kosic@...ibm.com>,
        "Alexei Starovoitov" <alexei.starovoitov@...il.com>,
        Li Zefan <lizefan@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: perf bpf: Reworked fix endianness problem when loading parameters
 in prologue

Hi Thomas,

Your patch looks good to me. I've tested in my environment and it works.
Please resend it to lkml and let Arnaldo to collect it.

Thank you.

On 2017/8/14 17:47, Thomas Richter wrote:
> Perf BPF prologue generator unconditionally fetches 8 bytes for function
> parameters, which causes problem on big endian machine. Thomas gives a
> detail analysis for this problem:
>
>   http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com
>
> This patch parses the type of each argument and converts data from
> memory to expected type.
>
> Now the test runs successfully on 4.13.0-rc5:
> [root@...60046 perf]# ./perf test  bpf
> 38: BPF filter                                 :
> 38.1: Basic BPF filtering                      : Ok
> 38.2: BPF pinning                              : Ok
> 38.3: BPF prologue generation                  : Ok
> 38.4: BPF relocation checker                   : Ok
> [root@...60046 perf]#
>
> Signed-off-by: Thomas Richter <tmricht@...ux.vnet.ibm.com>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Thomas Richter <tmricht@...ux.vnet.ibm.com>
> Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>
> Cc: Hendrik Brueckner <brueckner@...ux.vnet.ibm.com>
> Cc: Li Zefan <lizefan@...wei.com>
> ---
>   tools/perf/util/bpf-prologue.c | 49 ++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
> index 1356220..827f914 100644
> --- a/tools/perf/util/bpf-prologue.c
> +++ b/tools/perf/util/bpf-prologue.c
> @@ -58,6 +58,46 @@ check_pos(struct bpf_insn_pos *pos)
>   	return 0;
>   }
>   
> +/*
> + * Convert type string (u8/u16/u32/u64/s8/s16/s32/s64 ..., see
> + * Documentation/trace/kprobetrace.txt) to size field of BPF_LDX_MEM
> + * instruction (BPF_{B,H,W,DW}).
> + */
> +static int
> +argtype_to_ldx_size(const char *type)
> +{
> +	int arg_size = type ? atoi(&type[1]) : 64;
> +
> +	switch (arg_size) {
> +	case 8:
> +		return BPF_B;
> +	case 16:
> +		return BPF_H;
> +	case 32:
> +		return BPF_W;
> +	case 64:
> +	default:
> +		return BPF_DW;
> +	}
> +}
> +
> +static const char *
> +insn_sz_to_str(int insn_sz)
> +{
> +	switch (insn_sz) {
> +	case BPF_B:
> +		return "BPF_B";
> +	case BPF_H:
> +		return "BPF_H";
> +	case BPF_W:
> +		return "BPF_W";
> +	case BPF_DW:
> +		return "BPF_DW";
> +	default:
> +		return "UNKNOWN";
> +	}
> +}
> +
>   /* Give it a shorter name */
>   #define ins(i, p) append_insn((i), (p))
>   
> @@ -258,9 +298,14 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos,
>   	}
>   
>   	/* Final pass: read to registers */
> -	for (i = 0; i < nargs; i++)
> -		ins(BPF_LDX_MEM(BPF_DW, BPF_PROLOGUE_START_ARG_REG + i,
> +	for (i = 0; i < nargs; i++) {
> +		int insn_sz = (args[i].ref) ? argtype_to_ldx_size(args[i].type) : BPF_DW;
> +
> +		pr_debug("prologue: load arg %d, insn_sz is %s\n",
> +			 i, insn_sz_to_str(insn_sz));
> +		ins(BPF_LDX_MEM(insn_sz, BPF_PROLOGUE_START_ARG_REG + i,
>   				BPF_REG_FP, -BPF_REG_SIZE * (i + 1)), pos);
> +	}
>   
>   	ins(BPF_JMP_IMM(BPF_JA, BPF_REG_0, 0, JMP_TO_SUCCESS_CODE), pos);
>