Skip to content

Commit

Permalink
bpf: verifier: Allow single packet invalidation
Browse files Browse the repository at this point in the history
Previously there could only be one packet. Helper functions that may
modify the packet could simply invalidate all packets. Now that we
support multiple packets, we should allow helpers to invalidate specific
packets.

This is leaving the default global invalidation in place in case that's
still useful. All existing packets use the default id of '0', and could
be transitioned to the specific packet code with no change in behavior.

This also adds ARG_PTR_TO_PACKET, to allow packets to be passed to
helper functions at all. This is required to inform the verifier which
packets should be invalidated. Currenly only one packet is allowed per
helper.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
Signed-off-by: Paul Lawrence <paullawrence@google.com>
  • Loading branch information
drosen-google authored and intel-lab-lkp committed Sep 27, 2022
1 parent 0d4d45b commit 2be5946
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ enum bpf_arg_type {
ARG_PTR_TO_TIMER, /* pointer to bpf_timer */
ARG_PTR_TO_KPTR, /* pointer to referenced kptr */
ARG_PTR_TO_DYNPTR, /* pointer to bpf_dynptr. See bpf_type_flag for dynptr type */
ARG_PTR_TO_PACKET, /* pointer to packet */
__BPF_ARG_TYPE_MAX,

/* Extended arg_types. */
Expand Down
11 changes: 11 additions & 0 deletions include/linux/bpf_fuse.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright 2022 Google LLC.
*/

#ifndef _BPF_FUSE_H
#define _BPF_FUSE_H

bool bpf_helper_changes_one_pkt_data(void *func);

#endif /* _BPF_FUSE_H */
5 changes: 5 additions & 0 deletions kernel/bpf/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -2685,6 +2685,11 @@ bool __weak bpf_helper_changes_pkt_data(void *func)
return false;
}

bool __weak bpf_helper_changes_one_pkt_data(void *func)
{
return false;
}

/* Return TRUE if the JIT backend wants verifier to enable sub-register usage
* analysis code and wants explicit zero extension inserted by verifier.
* Otherwise, return FALSE.
Expand Down
83 changes: 81 additions & 2 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <linux/error-injection.h>
#include <linux/bpf_lsm.h>
#include <linux/btf_ids.h>
#include <linux/bpf_fuse.h>

#include "disasm.h"

Expand Down Expand Up @@ -263,6 +264,7 @@ struct bpf_call_arg_meta {
u32 subprogno;
struct bpf_map_value_off_desc *kptr_off_desc;
u8 uninit_dynptr_regno;
u32 data_id;
};

struct btf *btf_vmlinux;
Expand Down Expand Up @@ -1396,6 +1398,12 @@ static bool reg_is_pkt_pointer_any(const struct bpf_reg_state *reg)
reg->type == PTR_TO_PACKET_END;
}

static bool reg_is_specific_pkt_pointer_any(const struct bpf_reg_state *reg, u32 id)
{
return (reg_is_pkt_pointer(reg) ||
reg->type == PTR_TO_PACKET_END) && reg->data_id == id;
}

/* Unmodified PTR_TO_PACKET[_META,_END] register from ctx access. */
static bool reg_is_init_pkt_pointer(const struct bpf_reg_state *reg,
enum bpf_reg_type which)
Expand Down Expand Up @@ -5664,6 +5672,7 @@ static const struct bpf_reg_types stack_ptr_types = { .types = { PTR_TO_STACK }
static const struct bpf_reg_types const_str_ptr_types = { .types = { PTR_TO_MAP_VALUE } };
static const struct bpf_reg_types timer_types = { .types = { PTR_TO_MAP_VALUE } };
static const struct bpf_reg_types kptr_types = { .types = { PTR_TO_MAP_VALUE } };
static const struct bpf_reg_types packet_ptr_types = { .types = { PTR_TO_PACKET } };

static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = {
[ARG_PTR_TO_MAP_KEY] = &map_key_value_types,
Expand Down Expand Up @@ -5691,6 +5700,7 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = {
[ARG_PTR_TO_TIMER] = &timer_types,
[ARG_PTR_TO_KPTR] = &kptr_types,
[ARG_PTR_TO_DYNPTR] = &stack_ptr_types,
[ARG_PTR_TO_PACKET] = &packet_ptr_types,
};

static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
Expand Down Expand Up @@ -5800,7 +5810,8 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
/* Some of the argument types nevertheless require a
* zero register offset.
*/
if (base_type(arg_type) != ARG_PTR_TO_ALLOC_MEM)
if (base_type(arg_type) != ARG_PTR_TO_ALLOC_MEM &&
base_type(arg_type) != ARG_PTR_TO_PACKET)
return 0;
break;
/* All the rest must be rejected, except PTR_TO_BTF_ID which allows
Expand Down Expand Up @@ -6135,6 +6146,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
if (process_kptr_func(env, regno, meta))
return -EACCES;
break;
case ARG_PTR_TO_PACKET:
meta->data_id = reg->data_id;
break;
}

return err;
Expand Down Expand Up @@ -6509,13 +6523,36 @@ static bool check_btf_id_ok(const struct bpf_func_proto *fn)
return true;
}

static bool check_packet_ok(const struct bpf_func_proto *fn)
{
int count = 0;

if (fn->arg1_type == ARG_PTR_TO_PACKET)
count++;
if (fn->arg2_type == ARG_PTR_TO_PACKET)
count++;
if (fn->arg3_type == ARG_PTR_TO_PACKET)
count++;
if (fn->arg4_type == ARG_PTR_TO_PACKET)
count++;
if (fn->arg5_type == ARG_PTR_TO_PACKET)
count++;

/* We only support one arg being a packet at the moment,
* which is sufficient for the helper functions we have right now.
*/
return count <= 1;
}


static int check_func_proto(const struct bpf_func_proto *fn, int func_id,
struct bpf_call_arg_meta *meta)
{
return check_raw_mode_ok(fn) &&
check_arg_pair_ok(fn) &&
check_btf_id_ok(fn) &&
check_refcount_ok(fn, func_id) ? 0 : -EINVAL;
check_refcount_ok(fn, func_id) &&
check_packet_ok(fn) ? 0 : -EINVAL;
}

/* Packet data might have moved, any old PTR_TO_PACKET[_META,_END]
Expand All @@ -6539,6 +6576,25 @@ static void __clear_all_pkt_pointers(struct bpf_verifier_env *env,
}
}

static void __clear_specific_pkt_pointers(struct bpf_verifier_env *env,
struct bpf_func_state *state,
u32 data_id)
{
struct bpf_reg_state *regs = state->regs, *reg;
int i;

for (i = 0; i < MAX_BPF_REG; i++)
if (reg_is_specific_pkt_pointer_any(&regs[i], data_id))
mark_reg_unknown(env, regs, i);

bpf_for_each_spilled_reg(i, state, reg) {
if (!reg)
continue;
if (reg_is_specific_pkt_pointer_any(reg, data_id))
__mark_reg_unknown(env, reg);
}
}

static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
{
struct bpf_verifier_state *vstate = env->cur_state;
Expand All @@ -6548,6 +6604,15 @@ static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
__clear_all_pkt_pointers(env, vstate->frame[i]);
}

static void clear_specific_pkt_pointers(struct bpf_verifier_env *env, u32 data_id)
{
struct bpf_verifier_state *vstate = env->cur_state;
int i;

for (i = 0; i <= vstate->curframe; i++)
__clear_specific_pkt_pointers(env, vstate->frame[i], data_id);
}

enum {
AT_PKT_END = -1,
BEYOND_PKT_END = -2,
Expand Down Expand Up @@ -7187,6 +7252,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
struct bpf_call_arg_meta meta;
int insn_idx = *insn_idx_p;
bool changes_data;
bool changes_specific_data;
int i, err, func_id;

/* find function prototype */
Expand Down Expand Up @@ -7224,6 +7290,17 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
return -EINVAL;
}

changes_specific_data = bpf_helper_changes_one_pkt_data(fn->func);
if (changes_data && fn->arg1_type != ARG_PTR_TO_PACKET &&
fn->arg2_type != ARG_PTR_TO_PACKET &&
fn->arg3_type != ARG_PTR_TO_PACKET &&
fn->arg4_type != ARG_PTR_TO_PACKET &&
fn->arg5_type != ARG_PTR_TO_PACKET) {
verbose(env, "kernel subsystem misconfigured func %s#%d: no packet arg\n",
func_id_name(func_id), func_id);
return -EINVAL;
}

memset(&meta, 0, sizeof(meta));
meta.pkt_access = fn->pkt_access;

Expand Down Expand Up @@ -7534,6 +7611,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn

if (changes_data)
clear_all_pkt_pointers(env);
if (changes_specific_data)
clear_specific_pkt_pointers(env, meta.data_id);
return 0;
}

Expand Down

0 comments on commit 2be5946

Please sign in to comment.