Skip to content

Commit d5273fd

Browse files
committed
Merge tag 'bpf-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf
Pull bpf fixes from Alexei Starovoitov: - Fix how linked registers track zero extension of subregisters (Daniel Borkmann) - Fix unsound scalar fork for OR instructions (Daniel Wade) - Fix exception exit lock check for subprogs (Ihor Solodrai) - Fix undefined behavior in interpreter for SDIV/SMOD instructions (Jenny Guanni Qu) - Release module's BTF when module is unloaded (Kumar Kartikeya Dwivedi) - Fix constant blinding for PROBE_MEM32 instructions (Sachin Kumar) - Reset register ID for END instructions to prevent incorrect value tracking (Yazhou Tang) * tag 'bpf-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: selftests/bpf: Add a test cases for sync_linked_regs regarding zext propagation bpf: Fix sync_linked_regs regarding BPF_ADD_CONST32 zext propagation selftests/bpf: Add tests for maybe_fork_scalars() OR vs AND handling bpf: Fix unsound scalar forking in maybe_fork_scalars() for BPF_OR selftests/bpf: Add tests for sdiv32/smod32 with INT_MIN dividend bpf: Fix undefined behavior in interpreter sdiv/smod for INT_MIN selftests/bpf: Add tests for bpf_throw lock leak from subprogs bpf: Fix exception exit lock checking for subprogs bpf: Release module BTF IDR before module unload selftests/bpf: Fix pkg-config call on static builds bpf: Fix constant blinding for PROBE_MEM32 stores selftests/bpf: Add test for BPF_END register ID reset bpf: Reset register ID for BPF_END value tracking
2 parents ac57fa9 + 4a04d13 commit d5273fd

File tree

9 files changed

+416
-24
lines changed

9 files changed

+416
-24
lines changed

kernel/bpf/btf.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,7 +1787,16 @@ static void btf_free_id(struct btf *btf)
17871787
* of the _bh() version.
17881788
*/
17891789
spin_lock_irqsave(&btf_idr_lock, flags);
1790-
idr_remove(&btf_idr, btf->id);
1790+
if (btf->id) {
1791+
idr_remove(&btf_idr, btf->id);
1792+
/*
1793+
* Clear the id here to make this function idempotent, since it will get
1794+
* called a couple of times for module BTFs: on module unload, and then
1795+
* the final btf_put(). btf_alloc_id() starts IDs with 1, so we can use
1796+
* 0 as sentinel value.
1797+
*/
1798+
WRITE_ONCE(btf->id, 0);
1799+
}
17911800
spin_unlock_irqrestore(&btf_idr_lock, flags);
17921801
}
17931802

@@ -8115,7 +8124,7 @@ static void bpf_btf_show_fdinfo(struct seq_file *m, struct file *filp)
81158124
{
81168125
const struct btf *btf = filp->private_data;
81178126

8118-
seq_printf(m, "btf_id:\t%u\n", btf->id);
8127+
seq_printf(m, "btf_id:\t%u\n", READ_ONCE(btf->id));
81198128
}
81208129
#endif
81218130

@@ -8197,7 +8206,7 @@ int btf_get_info_by_fd(const struct btf *btf,
81978206
if (copy_from_user(&info, uinfo, info_copy))
81988207
return -EFAULT;
81998208

8200-
info.id = btf->id;
8209+
info.id = READ_ONCE(btf->id);
82018210
ubtf = u64_to_user_ptr(info.btf);
82028211
btf_copy = min_t(u32, btf->data_size, info.btf_size);
82038212
if (copy_to_user(ubtf, btf->data, btf_copy))
@@ -8260,7 +8269,7 @@ int btf_get_fd_by_id(u32 id)
82608269

82618270
u32 btf_obj_id(const struct btf *btf)
82628271
{
8263-
return btf->id;
8272+
return READ_ONCE(btf->id);
82648273
}
82658274

82668275
bool btf_is_kernel(const struct btf *btf)
@@ -8382,6 +8391,13 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
83828391
if (btf_mod->module != module)
83838392
continue;
83848393

8394+
/*
8395+
* For modules, we do the freeing of BTF IDR as soon as
8396+
* module goes away to disable BTF discovery, since the
8397+
* btf_try_get_module() on such BTFs will fail. This may
8398+
* be called again on btf_put(), but it's ok to do so.
8399+
*/
8400+
btf_free_id(btf_mod->btf);
83858401
list_del(&btf_mod->list);
83868402
if (btf_mod->sysfs_attr)
83878403
sysfs_remove_bin_file(btf_kobj, btf_mod->sysfs_attr);

kernel/bpf/core.c

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,27 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
14221422
*to++ = BPF_ALU64_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
14231423
*to++ = BPF_STX_MEM(from->code, from->dst_reg, BPF_REG_AX, from->off);
14241424
break;
1425+
1426+
case BPF_ST | BPF_PROBE_MEM32 | BPF_DW:
1427+
case BPF_ST | BPF_PROBE_MEM32 | BPF_W:
1428+
case BPF_ST | BPF_PROBE_MEM32 | BPF_H:
1429+
case BPF_ST | BPF_PROBE_MEM32 | BPF_B:
1430+
*to++ = BPF_ALU64_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^
1431+
from->imm);
1432+
*to++ = BPF_ALU64_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
1433+
/*
1434+
* Cannot use BPF_STX_MEM() macro here as it
1435+
* hardcodes BPF_MEM mode, losing PROBE_MEM32
1436+
* and breaking arena addressing in the JIT.
1437+
*/
1438+
*to++ = (struct bpf_insn) {
1439+
.code = BPF_STX | BPF_PROBE_MEM32 |
1440+
BPF_SIZE(from->code),
1441+
.dst_reg = from->dst_reg,
1442+
.src_reg = BPF_REG_AX,
1443+
.off = from->off,
1444+
};
1445+
break;
14251446
}
14261447
out:
14271448
return to - to_buff;
@@ -1736,6 +1757,12 @@ bool bpf_opcode_in_insntable(u8 code)
17361757
}
17371758

17381759
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
1760+
/* Absolute value of s32 without undefined behavior for S32_MIN */
1761+
static u32 abs_s32(s32 x)
1762+
{
1763+
return x >= 0 ? (u32)x : -(u32)x;
1764+
}
1765+
17391766
/**
17401767
* ___bpf_prog_run - run eBPF program on a given context
17411768
* @regs: is the array of MAX_BPF_EXT_REG eBPF pseudo-registers
@@ -1900,8 +1927,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
19001927
DST = do_div(AX, (u32) SRC);
19011928
break;
19021929
case 1:
1903-
AX = abs((s32)DST);
1904-
AX = do_div(AX, abs((s32)SRC));
1930+
AX = abs_s32((s32)DST);
1931+
AX = do_div(AX, abs_s32((s32)SRC));
19051932
if ((s32)DST < 0)
19061933
DST = (u32)-AX;
19071934
else
@@ -1928,8 +1955,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
19281955
DST = do_div(AX, (u32) IMM);
19291956
break;
19301957
case 1:
1931-
AX = abs((s32)DST);
1932-
AX = do_div(AX, abs((s32)IMM));
1958+
AX = abs_s32((s32)DST);
1959+
AX = do_div(AX, abs_s32((s32)IMM));
19331960
if ((s32)DST < 0)
19341961
DST = (u32)-AX;
19351962
else
@@ -1955,8 +1982,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
19551982
DST = (u32) AX;
19561983
break;
19571984
case 1:
1958-
AX = abs((s32)DST);
1959-
do_div(AX, abs((s32)SRC));
1985+
AX = abs_s32((s32)DST);
1986+
do_div(AX, abs_s32((s32)SRC));
19601987
if (((s32)DST < 0) == ((s32)SRC < 0))
19611988
DST = (u32)AX;
19621989
else
@@ -1982,8 +2009,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
19822009
DST = (u32) AX;
19832010
break;
19842011
case 1:
1985-
AX = abs((s32)DST);
1986-
do_div(AX, abs((s32)IMM));
2012+
AX = abs_s32((s32)DST);
2013+
do_div(AX, abs_s32((s32)IMM));
19872014
if (((s32)DST < 0) == ((s32)IMM < 0))
19882015
DST = (u32)AX;
19892016
else

kernel/bpf/verifier.c

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15910,6 +15910,13 @@ static void scalar_byte_swap(struct bpf_reg_state *dst_reg, struct bpf_insn *ins
1591015910
/* Apply bswap if alu64 or switch between big-endian and little-endian machines */
1591115911
bool need_bswap = alu64 || (to_le == is_big_endian);
1591215912

15913+
/*
15914+
* If the register is mutated, manually reset its scalar ID to break
15915+
* any existing ties and avoid incorrect bounds propagation.
15916+
*/
15917+
if (need_bswap || insn->imm == 16 || insn->imm == 32)
15918+
dst_reg->id = 0;
15919+
1591315920
if (need_bswap) {
1591415921
if (insn->imm == 16)
1591515922
dst_reg->var_off = tnum_bswap16(dst_reg->var_off);
@@ -15992,7 +15999,7 @@ static int maybe_fork_scalars(struct bpf_verifier_env *env, struct bpf_insn *ins
1599215999
else
1599316000
return 0;
1599416001

15995-
branch = push_stack(env, env->insn_idx + 1, env->insn_idx, false);
16002+
branch = push_stack(env, env->insn_idx, env->insn_idx, false);
1599616003
if (IS_ERR(branch))
1599716004
return PTR_ERR(branch);
1599816005

@@ -17408,6 +17415,12 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s
1740817415
continue;
1740917416
if ((reg->id & ~BPF_ADD_CONST) != (known_reg->id & ~BPF_ADD_CONST))
1741017417
continue;
17418+
/*
17419+
* Skip mixed 32/64-bit links: the delta relationship doesn't
17420+
* hold across different ALU widths.
17421+
*/
17422+
if (((reg->id ^ known_reg->id) & BPF_ADD_CONST) == BPF_ADD_CONST)
17423+
continue;
1741117424
if ((!(reg->id & BPF_ADD_CONST) && !(known_reg->id & BPF_ADD_CONST)) ||
1741217425
reg->off == known_reg->off) {
1741317426
s32 saved_subreg_def = reg->subreg_def;
@@ -17435,7 +17448,7 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s
1743517448
scalar32_min_max_add(reg, &fake_reg);
1743617449
scalar_min_max_add(reg, &fake_reg);
1743717450
reg->var_off = tnum_add(reg->var_off, fake_reg.var_off);
17438-
if (known_reg->id & BPF_ADD_CONST32)
17451+
if ((reg->id | known_reg->id) & BPF_ADD_CONST32)
1743917452
zext_32_to_64(reg);
1744017453
reg_bounds_sync(reg);
1744117454
}
@@ -19863,11 +19876,14 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
1986319876
* Also verify that new value satisfies old value range knowledge.
1986419877
*/
1986519878

19866-
/* ADD_CONST mismatch: different linking semantics */
19867-
if ((rold->id & BPF_ADD_CONST) && !(rcur->id & BPF_ADD_CONST))
19868-
return false;
19869-
19870-
if (rold->id && !(rold->id & BPF_ADD_CONST) && (rcur->id & BPF_ADD_CONST))
19879+
/*
19880+
* ADD_CONST flags must match exactly: BPF_ADD_CONST32 and
19881+
* BPF_ADD_CONST64 have different linking semantics in
19882+
* sync_linked_regs() (alu32 zero-extends, alu64 does not),
19883+
* so pruning across different flag types is unsafe.
19884+
*/
19885+
if (rold->id &&
19886+
(rold->id & BPF_ADD_CONST) != (rcur->id & BPF_ADD_CONST))
1987119887
return false;
1987219888

1987319889
/* Both have offset linkage: offsets must match */
@@ -20904,7 +20920,8 @@ static int process_bpf_exit_full(struct bpf_verifier_env *env,
2090420920
* state when it exits.
2090520921
*/
2090620922
int err = check_resource_leak(env, exception_exit,
20907-
!env->cur_state->curframe,
20923+
exception_exit || !env->cur_state->curframe,
20924+
exception_exit ? "bpf_throw" :
2090820925
"BPF_EXIT instruction in main prog");
2090920926
if (err)
2091020927
return err;

tools/testing/selftests/bpf/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ $(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids \
409409
CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
410410
LIBBPF_INCLUDE=$(HOST_INCLUDE_DIR) \
411411
EXTRA_LDFLAGS='$(SAN_LDFLAGS) $(EXTRA_LDFLAGS)' \
412-
HOSTPKG_CONFIG=$(PKG_CONFIG) \
412+
HOSTPKG_CONFIG='$(PKG_CONFIG)' \
413413
OUTPUT=$(HOST_BUILD_DIR)/resolve_btfids/ BPFOBJ=$(HOST_BPFOBJ)
414414

415415
# Get Clang's default includes on this system, as opposed to those seen by

tools/testing/selftests/bpf/progs/exceptions_fail.c

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
#include "bpf_experimental.h"
99

1010
extern void bpf_rcu_read_lock(void) __ksym;
11+
extern void bpf_rcu_read_unlock(void) __ksym;
12+
extern void bpf_preempt_disable(void) __ksym;
13+
extern void bpf_preempt_enable(void) __ksym;
14+
extern void bpf_local_irq_save(unsigned long *) __ksym;
15+
extern void bpf_local_irq_restore(unsigned long *) __ksym;
1116

1217
#define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
1318

@@ -131,7 +136,7 @@ int reject_subprog_with_lock(void *ctx)
131136
}
132137

133138
SEC("?tc")
134-
__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_rcu_read_lock-ed region")
139+
__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
135140
int reject_with_rcu_read_lock(void *ctx)
136141
{
137142
bpf_rcu_read_lock();
@@ -147,11 +152,13 @@ __noinline static int throwing_subprog(struct __sk_buff *ctx)
147152
}
148153

149154
SEC("?tc")
150-
__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_rcu_read_lock-ed region")
155+
__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
151156
int reject_subprog_with_rcu_read_lock(void *ctx)
152157
{
153158
bpf_rcu_read_lock();
154-
return throwing_subprog(ctx);
159+
throwing_subprog(ctx);
160+
bpf_rcu_read_unlock();
161+
return 0;
155162
}
156163

157164
static bool rbless(struct bpf_rb_node *n1, const struct bpf_rb_node *n2)
@@ -346,4 +353,47 @@ int reject_exception_throw_cb_diff(struct __sk_buff *ctx)
346353
return 0;
347354
}
348355

356+
__noinline static int always_throws(void)
357+
{
358+
bpf_throw(0);
359+
return 0;
360+
}
361+
362+
__noinline static int rcu_lock_then_throw(void)
363+
{
364+
bpf_rcu_read_lock();
365+
bpf_throw(0);
366+
return 0;
367+
}
368+
369+
SEC("?tc")
370+
__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
371+
int reject_subprog_rcu_lock_throw(void *ctx)
372+
{
373+
rcu_lock_then_throw();
374+
return 0;
375+
}
376+
377+
SEC("?tc")
378+
__failure __msg("bpf_throw cannot be used inside bpf_preempt_disable-ed region")
379+
int reject_subprog_throw_preempt_lock(void *ctx)
380+
{
381+
bpf_preempt_disable();
382+
always_throws();
383+
bpf_preempt_enable();
384+
return 0;
385+
}
386+
387+
SEC("?tc")
388+
__failure __msg("bpf_throw cannot be used inside bpf_local_irq_save-ed region")
389+
int reject_subprog_throw_irq_lock(void *ctx)
390+
{
391+
unsigned long flags;
392+
393+
bpf_local_irq_save(&flags);
394+
always_throws();
395+
bpf_local_irq_restore(&flags);
396+
return 0;
397+
}
398+
349399
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)