Skip to content

Commit

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

====================
Various BPF fixes

Follow-up to fix incorrect pruning when alignment tracking is
in use and to properly clear regs after call to not leave stale
data behind, also a fix that adds bpf_clone_redirect to the
bpf_helper_changes_pkt_data helper and exposes correct map_flags
for lpm map into fdinfo. For details, please see individual
patches.

v1 -> v2:
  - Reworked first patch so that env->strict_alignment is the
    final indicator on whether we have to deal with strict
    alignment rather than having CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
    checks on various locations, so only checking env->strict_alignment
    is sufficient after that. Thanks for spotting, Dave!
  - Added patch 3 and 4.
  - Rest as is.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed May 25, 2017
2 parents 5990baa + 614d0d7 commit ae08ea9
Show file tree
Hide file tree
Showing 8 changed files with 285 additions and 34 deletions.
10 changes: 10 additions & 0 deletions include/linux/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,16 @@ struct bpf_prog_aux;
.off = OFF, \
.imm = IMM })

/* Unconditional jumps, goto pc + off16 */

#define BPF_JMP_A(OFF) \
((struct bpf_insn) { \
.code = BPF_JMP | BPF_JA, \
.dst_reg = 0, \
.src_reg = 0, \
.off = OFF, \
.imm = 0 })

/* Function call */

#define BPF_EMIT_CALL(FUNC) \
Expand Down
1 change: 1 addition & 0 deletions kernel/bpf/arraymap.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
array->map.key_size = attr->key_size;
array->map.value_size = attr->value_size;
array->map.max_entries = attr->max_entries;
array->map.map_flags = attr->map_flags;
array->elem_size = elem_size;

if (!percpu)
Expand Down
1 change: 1 addition & 0 deletions kernel/bpf/lpm_trie.c
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ static struct bpf_map *trie_alloc(union bpf_attr *attr)
trie->map.key_size = attr->key_size;
trie->map.value_size = attr->value_size;
trie->map.max_entries = attr->max_entries;
trie->map.map_flags = attr->map_flags;
trie->data_size = attr->key_size -
offsetof(struct bpf_lpm_trie_key, data);
trie->max_prefixlen = trie->data_size * 8;
Expand Down
1 change: 1 addition & 0 deletions kernel/bpf/stackmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ static struct bpf_map *stack_map_alloc(union bpf_attr *attr)
smap->map.key_size = attr->key_size;
smap->map.value_size = value_size;
smap->map.max_entries = attr->max_entries;
smap->map.map_flags = attr->map_flags;
smap->n_buckets = n_buckets;
smap->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;

Expand Down
56 changes: 26 additions & 30 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,19 +463,22 @@ static const int caller_saved[CALLER_SAVED_REGS] = {
BPF_REG_0, BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5
};

static void mark_reg_not_init(struct bpf_reg_state *regs, u32 regno)
{
BUG_ON(regno >= MAX_BPF_REG);

memset(&regs[regno], 0, sizeof(regs[regno]));
regs[regno].type = NOT_INIT;
regs[regno].min_value = BPF_REGISTER_MIN_RANGE;
regs[regno].max_value = BPF_REGISTER_MAX_RANGE;
}

static void init_reg_state(struct bpf_reg_state *regs)
{
int i;

for (i = 0; i < MAX_BPF_REG; i++) {
regs[i].type = NOT_INIT;
regs[i].imm = 0;
regs[i].min_value = BPF_REGISTER_MIN_RANGE;
regs[i].max_value = BPF_REGISTER_MAX_RANGE;
regs[i].min_align = 0;
regs[i].aux_off = 0;
regs[i].aux_off_align = 0;
}
for (i = 0; i < MAX_BPF_REG; i++)
mark_reg_not_init(regs, i);

/* frame pointer */
regs[BPF_REG_FP].type = FRAME_PTR;
Expand Down Expand Up @@ -843,9 +846,6 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
{
bool strict = env->strict_alignment;

if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
strict = true;

switch (reg->type) {
case PTR_TO_PACKET:
return check_pkt_ptr_alignment(reg, off, size, strict);
Expand Down Expand Up @@ -1349,7 +1349,6 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
struct bpf_verifier_state *state = &env->cur_state;
const struct bpf_func_proto *fn = NULL;
struct bpf_reg_state *regs = state->regs;
struct bpf_reg_state *reg;
struct bpf_call_arg_meta meta;
bool changes_data;
int i, err;
Expand Down Expand Up @@ -1416,11 +1415,8 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
}

/* reset caller saved regs */
for (i = 0; i < CALLER_SAVED_REGS; i++) {
reg = regs + caller_saved[i];
reg->type = NOT_INIT;
reg->imm = 0;
}
for (i = 0; i < CALLER_SAVED_REGS; i++)
mark_reg_not_init(regs, caller_saved[i]);

/* update return register */
if (fn->ret_type == RET_INTEGER) {
Expand Down Expand Up @@ -2448,7 +2444,6 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
{
struct bpf_reg_state *regs = env->cur_state.regs;
u8 mode = BPF_MODE(insn->code);
struct bpf_reg_state *reg;
int i, err;

if (!may_access_skb(env->prog->type)) {
Expand Down Expand Up @@ -2481,11 +2476,8 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
}

/* reset caller saved regs to unreadable */
for (i = 0; i < CALLER_SAVED_REGS; i++) {
reg = regs + caller_saved[i];
reg->type = NOT_INIT;
reg->imm = 0;
}
for (i = 0; i < CALLER_SAVED_REGS; i++)
mark_reg_not_init(regs, caller_saved[i]);

/* mark destination R0 register as readable, since it contains
* the value fetched from the packet
Expand Down Expand Up @@ -2696,7 +2688,8 @@ static int check_cfg(struct bpf_verifier_env *env)
/* the following conditions reduce the number of explored insns
* from ~140k to ~80k for ultra large programs that use a lot of ptr_to_packet
*/
static bool compare_ptrs_to_packet(struct bpf_reg_state *old,
static bool compare_ptrs_to_packet(struct bpf_verifier_env *env,
struct bpf_reg_state *old,
struct bpf_reg_state *cur)
{
if (old->id != cur->id)
Expand Down Expand Up @@ -2739,7 +2732,7 @@ static bool compare_ptrs_to_packet(struct bpf_reg_state *old,
* 'if (R4 > data_end)' and all further insn were already good with r=20,
* so they will be good with r=30 and we can prune the search.
*/
if (old->off <= cur->off &&
if (!env->strict_alignment && old->off <= cur->off &&
old->off >= old->range && cur->off >= cur->range)
return true;

Expand Down Expand Up @@ -2810,7 +2803,7 @@ static bool states_equal(struct bpf_verifier_env *env,
continue;

if (rold->type == PTR_TO_PACKET && rcur->type == PTR_TO_PACKET &&
compare_ptrs_to_packet(rold, rcur))
compare_ptrs_to_packet(env, rold, rcur))
continue;

return false;
Expand Down Expand Up @@ -3588,10 +3581,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
} else {
log_level = 0;
}
if (attr->prog_flags & BPF_F_STRICT_ALIGNMENT)

env->strict_alignment = !!(attr->prog_flags & BPF_F_STRICT_ALIGNMENT);
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
env->strict_alignment = true;
else
env->strict_alignment = false;

ret = replace_map_fd_with_map_ptr(env);
if (ret < 0)
Expand Down Expand Up @@ -3697,7 +3690,10 @@ int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
mutex_lock(&bpf_verifier_lock);

log_level = 0;

env->strict_alignment = false;
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
env->strict_alignment = true;

env->explored_states = kcalloc(env->prog->len,
sizeof(struct bpf_verifier_state_list *),
Expand Down
1 change: 1 addition & 0 deletions net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -2281,6 +2281,7 @@ bool bpf_helper_changes_pkt_data(void *func)
func == bpf_skb_change_head ||
func == bpf_skb_change_tail ||
func == bpf_skb_pull_data ||
func == bpf_clone_redirect ||
func == bpf_l3_csum_replace ||
func == bpf_l4_csum_replace ||
func == bpf_xdp_adjust_head)
Expand Down
10 changes: 10 additions & 0 deletions tools/include/linux/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@
.off = OFF, \
.imm = IMM })

/* Unconditional jumps, goto pc + off16 */

#define BPF_JMP_A(OFF) \
((struct bpf_insn) { \
.code = BPF_JMP | BPF_JA, \
.dst_reg = 0, \
.src_reg = 0, \
.off = OFF, \
.imm = 0 })

/* Function call */

#define BPF_EMIT_CALL(FUNC) \
Expand Down
Loading

0 comments on commit ae08ea9

Please sign in to comment.