Commit f96da094 authored by Daniel Borkmann's avatar Daniel Borkmann Committed by David S. Miller

bpf: simplify narrower ctx access

This work tries to make the semantics and code around the
narrower ctx access a bit easier to follow. Right now
everything is done inside the .is_valid_access(). Offset
matching is done differently for read/write types, meaning
writes don't support narrower access and thus matching only
on offsetof(struct foo, bar) is enough whereas for read
case that supports narrower access we must check for
offsetof(struct foo, bar) + offsetof(struct foo, bar) +
sizeof(<bar>) - 1 for each of the cases. For read cases of
individual members that don't support narrower access (like
packet pointers or skb->cb[] case which has its own narrow
access logic), we check as usual only offsetof(struct foo,
bar) like in write case. Then, for the case where narrower
access is allowed, we also need to set the aux info for the
access. Meaning, ctx_field_size and converted_op_size have
to be set. First is the original field size e.g. sizeof(<bar>)
as in above example from the user facing ctx, and latter
one is the target size after actual rewrite happened, thus
for the kernel facing ctx. Also here we need the range match
and we need to keep track changing convert_ctx_access() and
converted_op_size from is_valid_access() as both are not at
the same location.

We can simplify the code a bit: check_ctx_access() becomes
simpler in that we only store ctx_field_size as a meta data
and later in convert_ctx_accesses() we fetch the target_size
right from the location where we do convert. Should the verifier
be misconfigured we do reject for BPF_WRITE cases or target_size
that are not provided. For the subsystems, we always work on
ranges in is_valid_access() and add small helpers for ranges
and narrow access, convert_ctx_accesses() sets target_size
for the relevant instruction.
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2be7e212
...@@ -156,9 +156,14 @@ struct bpf_prog; ...@@ -156,9 +156,14 @@ struct bpf_prog;
struct bpf_insn_access_aux { struct bpf_insn_access_aux {
enum bpf_reg_type reg_type; enum bpf_reg_type reg_type;
int ctx_field_size; int ctx_field_size;
int converted_op_size;
}; };
static inline void
bpf_ctx_record_field_size(struct bpf_insn_access_aux *aux, u32 size)
{
aux->ctx_field_size = size;
}
struct bpf_verifier_ops { struct bpf_verifier_ops {
/* return eBPF function prototype for verification */ /* return eBPF function prototype for verification */
const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id); const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
...@@ -173,7 +178,7 @@ struct bpf_verifier_ops { ...@@ -173,7 +178,7 @@ struct bpf_verifier_ops {
u32 (*convert_ctx_access)(enum bpf_access_type type, u32 (*convert_ctx_access)(enum bpf_access_type type,
const struct bpf_insn *src, const struct bpf_insn *src,
struct bpf_insn *dst, struct bpf_insn *dst,
struct bpf_prog *prog); struct bpf_prog *prog, u32 *target_size);
int (*test_run)(struct bpf_prog *prog, const union bpf_attr *kattr, int (*test_run)(struct bpf_prog *prog, const union bpf_attr *kattr,
union bpf_attr __user *uattr); union bpf_attr __user *uattr);
}; };
......
...@@ -337,6 +337,22 @@ struct bpf_prog_aux; ...@@ -337,6 +337,22 @@ struct bpf_prog_aux;
bpf_size; \ bpf_size; \
}) })
#define bpf_size_to_bytes(bpf_size) \
({ \
int bytes = -EINVAL; \
\
if (bpf_size == BPF_B) \
bytes = sizeof(u8); \
else if (bpf_size == BPF_H) \
bytes = sizeof(u16); \
else if (bpf_size == BPF_W) \
bytes = sizeof(u32); \
else if (bpf_size == BPF_DW) \
bytes = sizeof(u64); \
\
bytes; \
})
#define BPF_SIZEOF(type) \ #define BPF_SIZEOF(type) \
({ \ ({ \
const int __size = bytes_to_bpf_size(sizeof(type)); \ const int __size = bytes_to_bpf_size(sizeof(type)); \
...@@ -351,6 +367,13 @@ struct bpf_prog_aux; ...@@ -351,6 +367,13 @@ struct bpf_prog_aux;
__size; \ __size; \
}) })
#define BPF_LDST_BYTES(insn) \
({ \
const int __size = bpf_size_to_bytes(BPF_SIZE(insn->code)); \
WARN_ON(__size < 0); \
__size; \
})
#define __BPF_MAP_0(m, v, ...) v #define __BPF_MAP_0(m, v, ...) v
#define __BPF_MAP_1(m, v, t, a, ...) m(t, a) #define __BPF_MAP_1(m, v, t, a, ...) m(t, a)
#define __BPF_MAP_2(m, v, t, a, ...) m(t, a), __BPF_MAP_1(m, v, __VA_ARGS__) #define __BPF_MAP_2(m, v, t, a, ...) m(t, a), __BPF_MAP_1(m, v, __VA_ARGS__)
...@@ -401,6 +424,18 @@ struct bpf_prog_aux; ...@@ -401,6 +424,18 @@ struct bpf_prog_aux;
#define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__) #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
#define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__) #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
#define bpf_ctx_range(TYPE, MEMBER) \
offsetof(TYPE, MEMBER) ... offsetofend(TYPE, MEMBER) - 1
#define bpf_ctx_range_till(TYPE, MEMBER1, MEMBER2) \
offsetof(TYPE, MEMBER1) ... offsetofend(TYPE, MEMBER2) - 1
#define bpf_target_off(TYPE, MEMBER, SIZE, PTR_SIZE) \
({ \
BUILD_BUG_ON(FIELD_SIZEOF(TYPE, MEMBER) != (SIZE)); \
*(PTR_SIZE) = (SIZE); \
offsetof(TYPE, MEMBER); \
})
#ifdef CONFIG_COMPAT #ifdef CONFIG_COMPAT
/* A struct sock_filter is architecture independent. */ /* A struct sock_filter is architecture independent. */
struct compat_sock_fprog { struct compat_sock_fprog {
...@@ -564,6 +599,18 @@ static inline bool bpf_prog_was_classic(const struct bpf_prog *prog) ...@@ -564,6 +599,18 @@ static inline bool bpf_prog_was_classic(const struct bpf_prog *prog)
return prog->type == BPF_PROG_TYPE_UNSPEC; return prog->type == BPF_PROG_TYPE_UNSPEC;
} }
static inline bool
bpf_ctx_narrow_access_ok(u32 off, u32 size, const u32 size_default)
{
bool off_ok;
#ifdef __LITTLE_ENDIAN
off_ok = (off & (size_default - 1)) == 0;
#else
off_ok = (off & (size_default - 1)) + size == size_default;
#endif
return off_ok && size <= size_default && (size & (size - 1)) == 0;
}
#define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0])) #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
#ifdef CONFIG_ARCH_HAS_SET_MEMORY #ifdef CONFIG_ARCH_HAS_SET_MEMORY
......
...@@ -546,20 +546,6 @@ static int check_reg_arg(struct bpf_reg_state *regs, u32 regno, ...@@ -546,20 +546,6 @@ static int check_reg_arg(struct bpf_reg_state *regs, u32 regno,
return 0; return 0;
} }
static int bpf_size_to_bytes(int bpf_size)
{
if (bpf_size == BPF_W)
return 4;
else if (bpf_size == BPF_H)
return 2;
else if (bpf_size == BPF_B)
return 1;
else if (bpf_size == BPF_DW)
return 8;
else
return -EINVAL;
}
static bool is_spillable_regtype(enum bpf_reg_type type) static bool is_spillable_regtype(enum bpf_reg_type type)
{ {
switch (type) { switch (type) {
...@@ -761,7 +747,9 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off, ...@@ -761,7 +747,9 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, int size, static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, int size,
enum bpf_access_type t, enum bpf_reg_type *reg_type) enum bpf_access_type t, enum bpf_reg_type *reg_type)
{ {
struct bpf_insn_access_aux info = { .reg_type = *reg_type }; struct bpf_insn_access_aux info = {
.reg_type = *reg_type,
};
/* for analyzer ctx accesses are already validated and converted */ /* for analyzer ctx accesses are already validated and converted */
if (env->analyzer_ops) if (env->analyzer_ops)
...@@ -769,25 +757,14 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, ...@@ -769,25 +757,14 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
if (env->prog->aux->ops->is_valid_access && if (env->prog->aux->ops->is_valid_access &&
env->prog->aux->ops->is_valid_access(off, size, t, &info)) { env->prog->aux->ops->is_valid_access(off, size, t, &info)) {
/* a non zero info.ctx_field_size indicates: /* A non zero info.ctx_field_size indicates that this field is a
* . For this field, the prog type specific ctx conversion algorithm * candidate for later verifier transformation to load the whole
* only supports whole field access. * field and then apply a mask when accessed with a narrower
* . This ctx access is a candiate for later verifier transformation * access than actual ctx access size. A zero info.ctx_field_size
* to load the whole field and then apply a mask to get correct result. * will only allow for whole field access and rejects any other
* a non zero info.converted_op_size indicates perceived actual converted * type of narrower access.
* value width in convert_ctx_access. */
*/
if ((info.ctx_field_size && !info.converted_op_size) ||
(!info.ctx_field_size && info.converted_op_size)) {
verbose("verifier bug in is_valid_access prog type=%u off=%d size=%d\n",
env->prog->type, off, size);
return -EACCES;
}
if (info.ctx_field_size) {
env->insn_aux_data[insn_idx].ctx_field_size = info.ctx_field_size; env->insn_aux_data[insn_idx].ctx_field_size = info.ctx_field_size;
env->insn_aux_data[insn_idx].converted_op_size = info.converted_op_size;
}
*reg_type = info.reg_type; *reg_type = info.reg_type;
/* remember the offset of last byte accessed in ctx */ /* remember the offset of last byte accessed in ctx */
...@@ -3401,11 +3378,13 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of ...@@ -3401,11 +3378,13 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
static int convert_ctx_accesses(struct bpf_verifier_env *env) static int convert_ctx_accesses(struct bpf_verifier_env *env)
{ {
const struct bpf_verifier_ops *ops = env->prog->aux->ops; const struct bpf_verifier_ops *ops = env->prog->aux->ops;
int i, cnt, size, ctx_field_size, delta = 0;
const int insn_cnt = env->prog->len; const int insn_cnt = env->prog->len;
struct bpf_insn insn_buf[16], *insn; struct bpf_insn insn_buf[16], *insn;
struct bpf_prog *new_prog; struct bpf_prog *new_prog;
enum bpf_access_type type; enum bpf_access_type type;
int i, cnt, off, size, ctx_field_size, converted_op_size, is_narrower_load, delta = 0; bool is_narrower_load;
u32 target_size;
if (ops->gen_prologue) { if (ops->gen_prologue) {
cnt = ops->gen_prologue(insn_buf, env->seen_direct_write, cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
...@@ -3445,33 +3424,44 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) ...@@ -3445,33 +3424,44 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX) if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
continue; continue;
off = insn->off;
size = bpf_size_to_bytes(BPF_SIZE(insn->code));
ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size; ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
converted_op_size = env->insn_aux_data[i + delta].converted_op_size; size = BPF_LDST_BYTES(insn);
is_narrower_load = type == BPF_READ && size < ctx_field_size;
/* If the read access is a narrower load of the field, /* If the read access is a narrower load of the field,
* convert to a 4/8-byte load, to minimum program type specific * convert to a 4/8-byte load, to minimum program type specific
* convert_ctx_access changes. If conversion is successful, * convert_ctx_access changes. If conversion is successful,
* we will apply proper mask to the result. * we will apply proper mask to the result.
*/ */
is_narrower_load = size < ctx_field_size;
if (is_narrower_load) { if (is_narrower_load) {
int size_code = BPF_H; u32 off = insn->off;
u8 size_code;
if (type == BPF_WRITE) {
verbose("bpf verifier narrow ctx access misconfigured\n");
return -EINVAL;
}
size_code = BPF_H;
if (ctx_field_size == 4) if (ctx_field_size == 4)
size_code = BPF_W; size_code = BPF_W;
else if (ctx_field_size == 8) else if (ctx_field_size == 8)
size_code = BPF_DW; size_code = BPF_DW;
insn->off = off & ~(ctx_field_size - 1); insn->off = off & ~(ctx_field_size - 1);
insn->code = BPF_LDX | BPF_MEM | size_code; insn->code = BPF_LDX | BPF_MEM | size_code;
} }
cnt = ops->convert_ctx_access(type, insn, insn_buf, env->prog);
if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) { target_size = 0;
cnt = ops->convert_ctx_access(type, insn, insn_buf, env->prog,
&target_size);
if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
(ctx_field_size && !target_size)) {
verbose("bpf verifier is misconfigured\n"); verbose("bpf verifier is misconfigured\n");
return -EINVAL; return -EINVAL;
} }
if (is_narrower_load && size < converted_op_size) {
if (is_narrower_load && size < target_size) {
if (ctx_field_size <= 4) if (ctx_field_size <= 4)
insn_buf[cnt++] = BPF_ALU32_IMM(BPF_AND, insn->dst_reg, insn_buf[cnt++] = BPF_ALU32_IMM(BPF_AND, insn->dst_reg,
(1 << size * 8) - 1); (1 << size * 8) - 1);
......
...@@ -583,7 +583,8 @@ const struct bpf_verifier_ops tracepoint_prog_ops = { ...@@ -583,7 +583,8 @@ const struct bpf_verifier_ops tracepoint_prog_ops = {
static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type, static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
struct bpf_insn_access_aux *info) struct bpf_insn_access_aux *info)
{ {
int sample_period_off; const int size_sp = FIELD_SIZEOF(struct bpf_perf_event_data,
sample_period);
if (off < 0 || off >= sizeof(struct bpf_perf_event_data)) if (off < 0 || off >= sizeof(struct bpf_perf_event_data))
return false; return false;
...@@ -592,43 +593,35 @@ static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type ...@@ -592,43 +593,35 @@ static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type
if (off % size != 0) if (off % size != 0)
return false; return false;
/* permit 1, 2, 4 byte narrower and 8 normal read access to sample_period */ switch (off) {
sample_period_off = offsetof(struct bpf_perf_event_data, sample_period); case bpf_ctx_range(struct bpf_perf_event_data, sample_period):
if (off >= sample_period_off && off < sample_period_off + sizeof(__u64)) { bpf_ctx_record_field_size(info, size_sp);
int allowed; if (!bpf_ctx_narrow_access_ok(off, size, size_sp))
#ifdef __LITTLE_ENDIAN
allowed = (off & 0x7) == 0 && size <= 8 && (size & (size - 1)) == 0;
#else
allowed = ((off & 0x7) + size) == 8 && size <= 8 && (size & (size - 1)) == 0;
#endif
if (!allowed)
return false; return false;
info->ctx_field_size = 8; break;
info->converted_op_size = 8; default:
} else {
if (size != sizeof(long)) if (size != sizeof(long))
return false; return false;
} }
return true; return true;
} }
static u32 pe_prog_convert_ctx_access(enum bpf_access_type type, static u32 pe_prog_convert_ctx_access(enum bpf_access_type type,
const struct bpf_insn *si, const struct bpf_insn *si,
struct bpf_insn *insn_buf, struct bpf_insn *insn_buf,
struct bpf_prog *prog) struct bpf_prog *prog, u32 *target_size)
{ {
struct bpf_insn *insn = insn_buf; struct bpf_insn *insn = insn_buf;
switch (si->off) { switch (si->off) {
case offsetof(struct bpf_perf_event_data, sample_period): case offsetof(struct bpf_perf_event_data, sample_period):
BUILD_BUG_ON(FIELD_SIZEOF(struct perf_sample_data, period) != sizeof(u64));
*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_perf_event_data_kern, *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_perf_event_data_kern,
data), si->dst_reg, si->src_reg, data), si->dst_reg, si->src_reg,
offsetof(struct bpf_perf_event_data_kern, data)); offsetof(struct bpf_perf_event_data_kern, data));
*insn++ = BPF_LDX_MEM(BPF_DW, si->dst_reg, si->dst_reg, *insn++ = BPF_LDX_MEM(BPF_DW, si->dst_reg, si->dst_reg,
offsetof(struct perf_sample_data, period)); bpf_target_off(struct perf_sample_data, period, 8,
target_size));
break; break;
default: default:
*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_perf_event_data_kern, *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_perf_event_data_kern,
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment