Skip to content

Commit 5e43f89

Browse files
rdnaborkmann
authored andcommitted
bpf: Check attach type at prog load time
== The problem == There are use-cases when a program of some type can be attached to multiple attach points and those attach points must have different permissions to access context or to call helpers. E.g. context structure may have fields for both IPv4 and IPv6 but it doesn't make sense to read from / write to IPv6 field when attach point is somewhere in IPv4 stack. Same applies to BPF-helpers: it may make sense to call some helper from some attach point, but not from other for same prog type. == The solution == Introduce `expected_attach_type` field in in `struct bpf_attr` for `BPF_PROG_LOAD` command. If scenario described in "The problem" section is the case for some prog type, the field will be checked twice: 1) At load time prog type is checked to see if attach type for it must be known to validate program permissions correctly. Prog will be rejected with EINVAL if it's the case and `expected_attach_type` is not specified or has invalid value. 2) At attach time `attach_type` is compared with `expected_attach_type`, if prog type requires to have one, and, if they differ, attach will be rejected with EINVAL. The `expected_attach_type` is now available as part of `struct bpf_prog` in both `bpf_verifier_ops->is_valid_access()` and `bpf_verifier_ops->get_func_proto()` () and can be used to check context accesses and calls to helpers correspondingly. Initially the idea was discussed by Alexei Starovoitov <[email protected]> and Daniel Borkmann <[email protected]> here: https://marc.info/?l=linux-netdev&m=152107378717201&w=2 Signed-off-by: Andrey Ignatov <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]>
1 parent 807ae7d commit 5e43f89

File tree

8 files changed

+88
-29
lines changed

8 files changed

+88
-29
lines changed

include/linux/bpf.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,15 @@ struct bpf_prog_ops {
208208

209209
struct bpf_verifier_ops {
210210
/* return eBPF function prototype for verification */
211-
const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
211+
const struct bpf_func_proto *
212+
(*get_func_proto)(enum bpf_func_id func_id,
213+
const struct bpf_prog *prog);
212214

213215
/* return true if 'size' wide access at offset 'off' within bpf_context
214216
* with 'type' (read or write) is allowed
215217
*/
216218
bool (*is_valid_access)(int off, int size, enum bpf_access_type type,
219+
const struct bpf_prog *prog,
217220
struct bpf_insn_access_aux *info);
218221
int (*gen_prologue)(struct bpf_insn *insn, bool direct_write,
219222
const struct bpf_prog *prog);

include/linux/filter.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ struct bpf_prog {
469469
is_func:1, /* program is a bpf function */
470470
kprobe_override:1; /* Do we override a kprobe? */
471471
enum bpf_prog_type type; /* Type of BPF program */
472+
enum bpf_attach_type expected_attach_type; /* For some prog types */
472473
u32 len; /* Number of filter blocks */
473474
u32 jited_len; /* Size of jited insns in bytes */
474475
u8 tag[BPF_TAG_SIZE];

include/uapi/linux/bpf.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,11 @@ union bpf_attr {
296296
__u32 prog_flags;
297297
char prog_name[BPF_OBJ_NAME_LEN];
298298
__u32 prog_ifindex; /* ifindex of netdev to prep for */
299+
/* For some prog types expected attach type must be known at
300+
* load time to verify attach type specific parts of prog
301+
* (context accesses, allowed helpers, etc).
302+
*/
303+
__u32 expected_attach_type;
299304
};
300305

301306
struct { /* anonymous struct used by BPF_OBJ_* commands */

kernel/bpf/cgroup.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
545545
EXPORT_SYMBOL(__cgroup_bpf_check_dev_permission);
546546

547547
static const struct bpf_func_proto *
548-
cgroup_dev_func_proto(enum bpf_func_id func_id)
548+
cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
549549
{
550550
switch (func_id) {
551551
case BPF_FUNC_map_lookup_elem:
@@ -566,6 +566,7 @@ cgroup_dev_func_proto(enum bpf_func_id func_id)
566566

567567
static bool cgroup_dev_is_valid_access(int off, int size,
568568
enum bpf_access_type type,
569+
const struct bpf_prog *prog,
569570
struct bpf_insn_access_aux *info)
570571
{
571572
const int size_default = sizeof(__u32);

kernel/bpf/syscall.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1171,8 +1171,27 @@ struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
11711171
}
11721172
EXPORT_SYMBOL_GPL(bpf_prog_get_type_dev);
11731173

1174+
static int
1175+
bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
1176+
enum bpf_attach_type expected_attach_type)
1177+
{
1178+
/* There are currently no prog types that require specifying
1179+
* attach_type at load time.
1180+
*/
1181+
return 0;
1182+
}
1183+
1184+
static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
1185+
enum bpf_attach_type attach_type)
1186+
{
1187+
/* There are currently no prog types that require specifying
1188+
* attach_type at load time.
1189+
*/
1190+
return 0;
1191+
}
1192+
11741193
/* last field in 'union bpf_attr' used by this command */
1175-
#define BPF_PROG_LOAD_LAST_FIELD prog_ifindex
1194+
#define BPF_PROG_LOAD_LAST_FIELD expected_attach_type
11761195

11771196
static int bpf_prog_load(union bpf_attr *attr)
11781197
{
@@ -1209,11 +1228,16 @@ static int bpf_prog_load(union bpf_attr *attr)
12091228
!capable(CAP_SYS_ADMIN))
12101229
return -EPERM;
12111230

1231+
if (bpf_prog_load_check_attach_type(type, attr->expected_attach_type))
1232+
return -EINVAL;
1233+
12121234
/* plain bpf_prog allocation */
12131235
prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
12141236
if (!prog)
12151237
return -ENOMEM;
12161238

1239+
prog->expected_attach_type = attr->expected_attach_type;
1240+
12171241
prog->aux->offload_requested = !!attr->prog_ifindex;
12181242

12191243
err = security_bpf_prog_alloc(prog->aux);
@@ -1474,6 +1498,11 @@ static int bpf_prog_attach(const union bpf_attr *attr)
14741498
if (IS_ERR(prog))
14751499
return PTR_ERR(prog);
14761500

1501+
if (bpf_prog_attach_check_attach_type(prog, attr->attach_type)) {
1502+
bpf_prog_put(prog);
1503+
return -EINVAL;
1504+
}
1505+
14771506
cgrp = cgroup_get_from_fd(attr->target_fd);
14781507
if (IS_ERR(cgrp)) {
14791508
bpf_prog_put(prog);

kernel/bpf/verifier.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
13231323
};
13241324

13251325
if (env->ops->is_valid_access &&
1326-
env->ops->is_valid_access(off, size, t, &info)) {
1326+
env->ops->is_valid_access(off, size, t, env->prog, &info)) {
13271327
/* A non zero info.ctx_field_size indicates that this field is a
13281328
* candidate for later verifier transformation to load the whole
13291329
* field and then apply a mask when accessed with a narrower
@@ -2349,7 +2349,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
23492349
}
23502350

23512351
if (env->ops->get_func_proto)
2352-
fn = env->ops->get_func_proto(func_id);
2352+
fn = env->ops->get_func_proto(func_id, env->prog);
23532353
if (!fn) {
23542354
verbose(env, "unknown func %s#%d\n", func_id_name(func_id),
23552355
func_id);
@@ -5572,7 +5572,7 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
55725572
insn = new_prog->insnsi + i + delta;
55735573
}
55745574
patch_call_imm:
5575-
fn = env->ops->get_func_proto(insn->imm);
5575+
fn = env->ops->get_func_proto(insn->imm, env->prog);
55765576
/* all functions that have prototype and verifier allowed
55775577
* programs to call them, must be real in-kernel functions
55785578
*/

kernel/trace/bpf_trace.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,8 @@ static const struct bpf_func_proto bpf_probe_read_str_proto = {
524524
.arg3_type = ARG_ANYTHING,
525525
};
526526

527-
static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
527+
static const struct bpf_func_proto *
528+
tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
528529
{
529530
switch (func_id) {
530531
case BPF_FUNC_map_lookup_elem:
@@ -568,7 +569,8 @@ static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
568569
}
569570
}
570571

571-
static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func_id)
572+
static const struct bpf_func_proto *
573+
kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
572574
{
573575
switch (func_id) {
574576
case BPF_FUNC_perf_event_output:
@@ -582,12 +584,13 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
582584
return &bpf_override_return_proto;
583585
#endif
584586
default:
585-
return tracing_func_proto(func_id);
587+
return tracing_func_proto(func_id, prog);
586588
}
587589
}
588590

589591
/* bpf+kprobe programs can access fields of 'struct pt_regs' */
590592
static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
593+
const struct bpf_prog *prog,
591594
struct bpf_insn_access_aux *info)
592595
{
593596
if (off < 0 || off >= sizeof(struct pt_regs))
@@ -661,19 +664,21 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
661664
.arg3_type = ARG_ANYTHING,
662665
};
663666

664-
static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
667+
static const struct bpf_func_proto *
668+
tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
665669
{
666670
switch (func_id) {
667671
case BPF_FUNC_perf_event_output:
668672
return &bpf_perf_event_output_proto_tp;
669673
case BPF_FUNC_get_stackid:
670674
return &bpf_get_stackid_proto_tp;
671675
default:
672-
return tracing_func_proto(func_id);
676+
return tracing_func_proto(func_id, prog);
673677
}
674678
}
675679

676680
static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type,
681+
const struct bpf_prog *prog,
677682
struct bpf_insn_access_aux *info)
678683
{
679684
if (off < sizeof(void *) || off >= PERF_MAX_TRACE_SIZE)
@@ -721,7 +726,8 @@ static const struct bpf_func_proto bpf_perf_prog_read_value_proto = {
721726
.arg3_type = ARG_CONST_SIZE,
722727
};
723728

724-
static const struct bpf_func_proto *pe_prog_func_proto(enum bpf_func_id func_id)
729+
static const struct bpf_func_proto *
730+
pe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
725731
{
726732
switch (func_id) {
727733
case BPF_FUNC_perf_event_output:
@@ -731,7 +737,7 @@ static const struct bpf_func_proto *pe_prog_func_proto(enum bpf_func_id func_id)
731737
case BPF_FUNC_perf_prog_read_value:
732738
return &bpf_perf_prog_read_value_proto;
733739
default:
734-
return tracing_func_proto(func_id);
740+
return tracing_func_proto(func_id, prog);
735741
}
736742
}
737743

@@ -781,20 +787,22 @@ static const struct bpf_func_proto bpf_get_stackid_proto_raw_tp = {
781787
.arg3_type = ARG_ANYTHING,
782788
};
783789

784-
static const struct bpf_func_proto *raw_tp_prog_func_proto(enum bpf_func_id func_id)
790+
static const struct bpf_func_proto *
791+
raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
785792
{
786793
switch (func_id) {
787794
case BPF_FUNC_perf_event_output:
788795
return &bpf_perf_event_output_proto_raw_tp;
789796
case BPF_FUNC_get_stackid:
790797
return &bpf_get_stackid_proto_raw_tp;
791798
default:
792-
return tracing_func_proto(func_id);
799+
return tracing_func_proto(func_id, prog);
793800
}
794801
}
795802

796803
static bool raw_tp_prog_is_valid_access(int off, int size,
797804
enum bpf_access_type type,
805+
const struct bpf_prog *prog,
798806
struct bpf_insn_access_aux *info)
799807
{
800808
/* largest tracepoint in the kernel has 12 args */
@@ -816,6 +824,7 @@ const struct bpf_prog_ops raw_tracepoint_prog_ops = {
816824
};
817825

818826
static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
827+
const struct bpf_prog *prog,
819828
struct bpf_insn_access_aux *info)
820829
{
821830
const int size_u64 = sizeof(u64);

0 commit comments

Comments
 (0)