netdev - Re: [PATCH bpf-next v4 01/16] bpf: implement lookup-free direct value access for maps

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 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:   Fri, 5 Apr 2019 18:56:23 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...nel.org,
        joe@...d.net.nz, yhs@...com, andrii.nakryiko@...il.com,
        kafai@...com
Subject: Re: [PATCH bpf-next v4 01/16] bpf: implement lookup-free direct
 value access for maps

On Fri, Apr 05, 2019 at 10:59:27PM +0200, Daniel Borkmann wrote:
>  
> -/* when bpf_ldimm64->src_reg == BPF_PSEUDO_MAP_FD, bpf_ldimm64->imm == fd */
> +/* When BPF ldimm64's insn[0].src_reg != 0 then this can have
> + * two extensions:
> + *
> + * insn[0].src_reg:  BPF_PSEUDO_MAP_FD   BPF_PSEUDO_MAP_VALUE
> + * insn[0].imm:      map fd              map fd
> + * insn[1].imm:      0                   offset into value
> + * insn[0].off:      0                   lower 16 bit of map index
> + * insn[1].off:      0                   higher 16 bit of map index
> + * ldimm64 rewrite:  address of map      address of map[index]+offset
> + * verifier type:    CONST_PTR_TO_MAP    PTR_TO_MAP_VALUE
...
> +	else if (insn->src_reg == BPF_PSEUDO_MAP_VALUE)
> +		snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> +			 "map[id:%u][%u]+%u", insn->imm,
> +			 ((__u32)(__u16)insn[0].off) |
> +			 ((__u32)(__u16)insn[1].off) << 16,
> +			 (insn + 1)->imm);

Hopefully one last nit...
Do we really need to allow this odd split index support?
Later patches enforce array of 1 element and libbpf only uses that.
This index feature feels too quirky and not really useful at this moment.
Can we enforce that insn[0|1].off == 0 instead ?
Later we can extend it to mean index without breaking anything.