Hi Joe,
On 09/28/2018 01:26 AM, Joe Stringer wrote:
> Teach the verifier a little bit about a new type of pointer, a
> PTR_TO_SOCKET. This pointer type is accessed from BPF through the
> 'struct bpf_sock' structure.
>
> Signed-off-by: Joe Stringer <[email protected]>
[...]
> }
> @@ -1726,6 +1755,14 @@ static int check_mem_access(struct bpf_verifier_env
> *env, int insn_idx, u32 regn
> err = check_flow_keys_access(env, off, size);
> if (!err && t == BPF_READ && value_regno >= 0)
> mark_reg_unknown(env, regs, value_regno);
> + } else if (reg->type == PTR_TO_SOCKET) {
> + if (t == BPF_WRITE) {
> + verbose(env, "cannot write into socket\n");
> + return -EACCES;
> + }
> + err = check_sock_access(env, regno, off, size, t);
> + if (!err && value_regno >= 0)
> + mark_reg_unknown(env, regs, value_regno);
Not an issue today, but this is quite fragile and very easy to miss, if we
allow to enable writes into ptr_to_socket in future e.g. mark or others,
then lifting above will not be enough. E.g. see check_xadd() and friends,
this rejects writes to ctx via f37a8cb84cce ("bpf: reject stores into ctx
via st and xadd") as otherwise this would bypass the context rewriter. So
I think we should add PTR_TO_SOCKET to is_ctx_reg() as well to have a full
guarantee this won't happen.
> } else {
> verbose(env, "R%d invalid mem access '%s'\n", regno,
> reg_type_str[reg->type]);
Thanks,
Daniel
