Skip to content

Commit

Permalink
Merge branch 'bpf-fixes'
Browse files Browse the repository at this point in the history
Alexei Starovoitov says:

====================
bpf fixes

Fixes for two bpf bugs:
1st bug reported by Sasha Goldshtein here:
https://github.com/iovisor/bcc/issues/570
2nd discovered by Daniel Borkmann by manual code analysis.
See patches for details.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Jun 16, 2016
2 parents e582615 + ad572d1 commit 8c08c73
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 39 deletions.
28 changes: 27 additions & 1 deletion include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,31 @@ enum bpf_access_type {
BPF_WRITE = 2
};

/* types of values stored in eBPF registers */
enum bpf_reg_type {
NOT_INIT = 0, /* nothing was written into register */
UNKNOWN_VALUE, /* reg doesn't contain a valid pointer */
PTR_TO_CTX, /* reg points to bpf_context */
CONST_PTR_TO_MAP, /* reg points to struct bpf_map */
PTR_TO_MAP_VALUE, /* reg points to map element value */
PTR_TO_MAP_VALUE_OR_NULL,/* points to map elem value or NULL */
FRAME_PTR, /* reg == frame_pointer */
PTR_TO_STACK, /* reg == frame_pointer + imm */
CONST_IMM, /* constant integer value */

/* PTR_TO_PACKET represents:
* skb->data
* skb->data + imm
* skb->data + (u16) var
* skb->data + (u16) var + imm
* if (range > 0) then [ptr, ptr + range - off) is safe to access
* if (id > 0) means that some 'var' was added
* if (off > 0) menas that 'imm' was added
*/
PTR_TO_PACKET,
PTR_TO_PACKET_END, /* skb->data + headlen */
};

struct bpf_prog;

struct bpf_verifier_ops {
Expand All @@ -120,7 +145,8 @@ struct bpf_verifier_ops {
/* return true if 'size' wide access at offset 'off' within bpf_context
* with 'type' (read or write) is allowed
*/
bool (*is_valid_access)(int off, int size, enum bpf_access_type type);
bool (*is_valid_access)(int off, int size, enum bpf_access_type type,
enum bpf_reg_type *reg_type);

u32 (*convert_ctx_access)(enum bpf_access_type type, int dst_reg,
int src_reg, int ctx_off,
Expand Down
41 changes: 7 additions & 34 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,31 +126,6 @@
* are set to NOT_INIT to indicate that they are no longer readable.
*/

/* types of values stored in eBPF registers */
enum bpf_reg_type {
NOT_INIT = 0, /* nothing was written into register */
UNKNOWN_VALUE, /* reg doesn't contain a valid pointer */
PTR_TO_CTX, /* reg points to bpf_context */
CONST_PTR_TO_MAP, /* reg points to struct bpf_map */
PTR_TO_MAP_VALUE, /* reg points to map element value */
PTR_TO_MAP_VALUE_OR_NULL,/* points to map elem value or NULL */
FRAME_PTR, /* reg == frame_pointer */
PTR_TO_STACK, /* reg == frame_pointer + imm */
CONST_IMM, /* constant integer value */

/* PTR_TO_PACKET represents:
* skb->data
* skb->data + imm
* skb->data + (u16) var
* skb->data + (u16) var + imm
* if (range > 0) then [ptr, ptr + range - off) is safe to access
* if (id > 0) means that some 'var' was added
* if (off > 0) menas that 'imm' was added
*/
PTR_TO_PACKET,
PTR_TO_PACKET_END, /* skb->data + headlen */
};

struct reg_state {
enum bpf_reg_type type;
union {
Expand Down Expand Up @@ -695,10 +670,10 @@ static int check_packet_access(struct verifier_env *env, u32 regno, int off,

/* check access to 'struct bpf_context' fields */
static int check_ctx_access(struct verifier_env *env, int off, int size,
enum bpf_access_type t)
enum bpf_access_type t, enum bpf_reg_type *reg_type)
{
if (env->prog->aux->ops->is_valid_access &&
env->prog->aux->ops->is_valid_access(off, size, t)) {
env->prog->aux->ops->is_valid_access(off, size, t, reg_type)) {
/* remember the offset of last byte accessed in ctx */
if (env->prog->aux->max_ctx_offset < off + size)
env->prog->aux->max_ctx_offset = off + size;
Expand Down Expand Up @@ -798,21 +773,19 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
mark_reg_unknown_value(state->regs, value_regno);

} else if (reg->type == PTR_TO_CTX) {
enum bpf_reg_type reg_type = UNKNOWN_VALUE;

if (t == BPF_WRITE && value_regno >= 0 &&
is_pointer_value(env, value_regno)) {
verbose("R%d leaks addr into ctx\n", value_regno);
return -EACCES;
}
err = check_ctx_access(env, off, size, t);
err = check_ctx_access(env, off, size, t, &reg_type);
if (!err && t == BPF_READ && value_regno >= 0) {
mark_reg_unknown_value(state->regs, value_regno);
if (off == offsetof(struct __sk_buff, data) &&
env->allow_ptr_leaks)
if (env->allow_ptr_leaks)
/* note that reg.[id|off|range] == 0 */
state->regs[value_regno].type = PTR_TO_PACKET;
else if (off == offsetof(struct __sk_buff, data_end) &&
env->allow_ptr_leaks)
state->regs[value_regno].type = PTR_TO_PACKET_END;
state->regs[value_regno].type = reg_type;
}

} else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
Expand Down
10 changes: 8 additions & 2 deletions kernel/trace/bpf_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ static u64 bpf_perf_event_read(u64 r1, u64 index, u64 r3, u64 r4, u64 r5)
event->pmu->count)
return -EINVAL;

if (unlikely(event->attr.type != PERF_TYPE_HARDWARE &&
event->attr.type != PERF_TYPE_RAW))
return -EINVAL;

/*
* we don't know if the function is run successfully by the
* return value. It can be judged in other places, such as
Expand Down Expand Up @@ -349,7 +353,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
}

/* bpf+kprobe programs can access fields of 'struct pt_regs' */
static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type type)
static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
enum bpf_reg_type *reg_type)
{
/* check bounds */
if (off < 0 || off >= sizeof(struct pt_regs))
Expand Down Expand Up @@ -427,7 +432,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
}
}

static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type)
static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type,
enum bpf_reg_type *reg_type)
{
if (off < sizeof(void *) || off >= PERF_MAX_TRACE_SIZE)
return false;
Expand Down
16 changes: 14 additions & 2 deletions net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,8 @@ static bool __is_valid_access(int off, int size, enum bpf_access_type type)
}

static bool sk_filter_is_valid_access(int off, int size,
enum bpf_access_type type)
enum bpf_access_type type,
enum bpf_reg_type *reg_type)
{
switch (off) {
case offsetof(struct __sk_buff, tc_classid):
Expand All @@ -2108,7 +2109,8 @@ static bool sk_filter_is_valid_access(int off, int size,
}

static bool tc_cls_act_is_valid_access(int off, int size,
enum bpf_access_type type)
enum bpf_access_type type,
enum bpf_reg_type *reg_type)
{
if (type == BPF_WRITE) {
switch (off) {
Expand All @@ -2123,6 +2125,16 @@ static bool tc_cls_act_is_valid_access(int off, int size,
return false;
}
}

switch (off) {
case offsetof(struct __sk_buff, data):
*reg_type = PTR_TO_PACKET;
break;
case offsetof(struct __sk_buff, data_end):
*reg_type = PTR_TO_PACKET_END;
break;
}

return __is_valid_access(off, size, type);
}

Expand Down

0 comments on commit 8c08c73

Please sign in to comment.