Skip to content

Commit 201b62c

Browse files
ameryhungAlexei Starovoitov
authored andcommitted
bpf: Refactor check_ctx_access()
Reduce the variable passing madness surrounding check_ctx_access(). Currently, check_mem_access() passes many pointers to local variables to check_ctx_access(). They are used to initialize "struct bpf_insn_access_aux info" in check_ctx_access() and then passed to is_valid_access(). Then, check_ctx_access() takes the data our from info and write them back the pointers to pass them back. This can be simpilified by moving info up to check_mem_access(). No functional change. Signed-off-by: Amery Hung <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 63817c7 commit 201b62c

File tree

1 file changed

+20
-36
lines changed

1 file changed

+20
-36
lines changed

kernel/bpf/verifier.c

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6051,42 +6051,26 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
60516051

60526052
/* check access to 'struct bpf_context' fields. Supports fixed offsets only */
60536053
static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, int size,
6054-
enum bpf_access_type t, enum bpf_reg_type *reg_type,
6055-
struct btf **btf, u32 *btf_id, bool *is_retval, bool is_ldsx,
6056-
u32 *ref_obj_id)
6054+
enum bpf_access_type t, struct bpf_insn_access_aux *info)
60576055
{
6058-
struct bpf_insn_access_aux info = {
6059-
.reg_type = *reg_type,
6060-
.log = &env->log,
6061-
.is_retval = false,
6062-
.is_ldsx = is_ldsx,
6063-
};
6064-
60656056
if (env->ops->is_valid_access &&
6066-
env->ops->is_valid_access(off, size, t, env->prog, &info)) {
6057+
env->ops->is_valid_access(off, size, t, env->prog, info)) {
60676058
/* A non zero info.ctx_field_size indicates that this field is a
60686059
* candidate for later verifier transformation to load the whole
60696060
* field and then apply a mask when accessed with a narrower
60706061
* access than actual ctx access size. A zero info.ctx_field_size
60716062
* will only allow for whole field access and rejects any other
60726063
* type of narrower access.
60736064
*/
6074-
*reg_type = info.reg_type;
6075-
*is_retval = info.is_retval;
6076-
6077-
if (base_type(*reg_type) == PTR_TO_BTF_ID) {
6078-
if (info.ref_obj_id &&
6079-
!find_reference_state(env->cur_state, info.ref_obj_id)) {
6065+
if (base_type(info->reg_type) == PTR_TO_BTF_ID) {
6066+
if (info->ref_obj_id &&
6067+
!find_reference_state(env->cur_state, info->ref_obj_id)) {
60806068
verbose(env, "invalid bpf_context access off=%d. Reference may already be released\n",
60816069
off);
60826070
return -EACCES;
60836071
}
6084-
6085-
*btf = info.btf;
6086-
*btf_id = info.btf_id;
6087-
*ref_obj_id = info.ref_obj_id;
60886072
} else {
6089-
env->insn_aux_data[insn_idx].ctx_field_size = info.ctx_field_size;
6073+
env->insn_aux_data[insn_idx].ctx_field_size = info->ctx_field_size;
60906074
}
60916075
/* remember the offset of last byte accessed in ctx */
60926076
if (env->prog->aux->max_ctx_offset < off + size)
@@ -7443,11 +7427,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
74437427
if (!err && value_regno >= 0 && (t == BPF_READ || rdonly_mem))
74447428
mark_reg_unknown(env, regs, value_regno);
74457429
} else if (reg->type == PTR_TO_CTX) {
7446-
bool is_retval = false;
74477430
struct bpf_retval_range range;
7448-
enum bpf_reg_type reg_type = SCALAR_VALUE;
7449-
struct btf *btf = NULL;
7450-
u32 btf_id = 0, ref_obj_id = 0;
7431+
struct bpf_insn_access_aux info = {
7432+
.reg_type = SCALAR_VALUE,
7433+
.is_ldsx = is_ldsx,
7434+
.log = &env->log,
7435+
};
74517436

74527437
if (t == BPF_WRITE && value_regno >= 0 &&
74537438
is_pointer_value(env, value_regno)) {
@@ -7459,17 +7444,16 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
74597444
if (err < 0)
74607445
return err;
74617446

7462-
err = check_ctx_access(env, insn_idx, off, size, t, &reg_type, &btf,
7463-
&btf_id, &is_retval, is_ldsx, &ref_obj_id);
7447+
err = check_ctx_access(env, insn_idx, off, size, t, &info);
74647448
if (err)
74657449
verbose_linfo(env, insn_idx, "; ");
74667450
if (!err && t == BPF_READ && value_regno >= 0) {
74677451
/* ctx access returns either a scalar, or a
74687452
* PTR_TO_PACKET[_META,_END]. In the latter
74697453
* case, we know the offset is zero.
74707454
*/
7471-
if (reg_type == SCALAR_VALUE) {
7472-
if (is_retval && get_func_retval_range(env->prog, &range)) {
7455+
if (info.reg_type == SCALAR_VALUE) {
7456+
if (info.is_retval && get_func_retval_range(env->prog, &range)) {
74737457
err = __mark_reg_s32_range(env, regs, value_regno,
74747458
range.minval, range.maxval);
74757459
if (err)
@@ -7480,21 +7464,21 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
74807464
} else {
74817465
mark_reg_known_zero(env, regs,
74827466
value_regno);
7483-
if (type_may_be_null(reg_type))
7467+
if (type_may_be_null(info.reg_type))
74847468
regs[value_regno].id = ++env->id_gen;
74857469
/* A load of ctx field could have different
74867470
* actual load size with the one encoded in the
74877471
* insn. When the dst is PTR, it is for sure not
74887472
* a sub-register.
74897473
*/
74907474
regs[value_regno].subreg_def = DEF_NOT_SUBREG;
7491-
if (base_type(reg_type) == PTR_TO_BTF_ID) {
7492-
regs[value_regno].btf = btf;
7493-
regs[value_regno].btf_id = btf_id;
7494-
regs[value_regno].ref_obj_id = ref_obj_id;
7475+
if (base_type(info.reg_type) == PTR_TO_BTF_ID) {
7476+
regs[value_regno].btf = info.btf;
7477+
regs[value_regno].btf_id = info.btf_id;
7478+
regs[value_regno].ref_obj_id = info.ref_obj_id;
74957479
}
74967480
}
7497-
regs[value_regno].type = reg_type;
7481+
regs[value_regno].type = info.reg_type;
74987482
}
74997483

75007484
} else if (reg->type == PTR_TO_STACK) {

0 commit comments

Comments
 (0)