netdev - Re: [PATCH RFC] sparc64: eBPF JIT

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:   Mon, 17 Apr 2017 18:12:45 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     alexei.starovoitov@...il.com
Cc:     sparclinux@...r.kernel.org, netdev@...r.kernel.org, ast@...nel.org,
        daniel@...earbox.net
Subject: Re: [PATCH RFC] sparc64: eBPF JIT

From: Alexei Starovoitov <alexei.starovoitov@...il.com>
Date: Mon, 17 Apr 2017 16:27:42 -0700

> On Sun, Apr 16, 2017 at 11:38:25PM -0400, David Miller wrote:
>> +static void build_prologue(struct jit_ctx *ctx)
>> +{
>> +	s32 stack_needed = 176;
>> +
>> +	if (ctx->saw_frame_pointer)
>> +		stack_needed += MAX_BPF_STACK;
>> +
>> +	/* save %sp, -176, %sp */
>> +	emit(SAVE | IMMED | RS1(SP) | S13(-stack_needed) | RD(SP), ctx);
>> +
>> +	if (ctx->saw_ld_abs_ind) {
>> +		load_skb_regs(ctx, bpf2sparc[BPF_REG_1]);
>> +	} else {
>> +		emit_nop(ctx);
>> +		emit_nop(ctx);
>> +		emit_nop(ctx);
>> +		emit_nop(ctx);
> 
> why 4 nops? to keep prologue size constant w/ and w/o caching ?
> does it help somehow? I'm assuming that's prep for next step
> of tail_call.

I need to make some adjustments to how the branch offsets are done
if the prologue is variable size.  Simply an implementation issue,
which I intend to fix, nothing more.

> 
>> +	if (insn->src_reg == BPF_REG_FP || insn->dst_reg == BPF_REG_FP) {
>> +		ctx->saw_frame_pointer = true;
>> +		if (BPF_CLASS(code) == BPF_ALU ||
>> +		    BPF_CLASS(code) == BPF_ALU64) {
>> +			pr_err_once("ALU op on FP not supported by JIT\n");
>> +			return -EINVAL;
> 
> That should be fine. The verifier checks for that:
>   /* check whether register used as dest operand can be written to */
>   if (regno == BPF_REG_FP) {
>           verbose("frame pointer is read only\n");
>           return -EACCES;
>   }

I need to trap it as a source as well, because if that is allowed then
I have to add special handling to every ALU operation we allow.

The reason is that the sparc64 stack is biased by 2047 bytes.  So
I have to adjust every FP relative reference to include that 2047
bias.

Can LLVM and CLANG emit arithmetic operations with FP as source?

>> +	/* dst = imm64 */
>> +	case BPF_LD | BPF_IMM | BPF_DW:
>> +	{
>> +		const struct bpf_insn insn1 = insn[1];
>> +		u64 imm64;
>> +
>> +		if (insn1.code != 0 || insn1.src_reg != 0 ||
>> +		    insn1.dst_reg != 0 || insn1.off != 0) {
>> +			/* Note: verifier in BPF core must catch invalid
>> +			 * instructions.
>> +			 */
>> +			pr_err_once("Invalid BPF_LD_IMM64 instruction\n");
>> +			return -EINVAL;
> 
> verifier should catch that too, but extra check doesn't hurt.

I just copied from anoter JIT, I can remove it.

> all looks great to me.

Thanks for reviewing.

>