Skip to content

Commit 11369e6

Browse files
author
Alexei Starovoitov
committed
Merge branch 'bpf-skip-bounds-adjustment-for-conditional-jumps-on-same-scalar-register'
KaFai Wan says: ==================== bpf: Skip bounds adjustment for conditional jumps on same scalar register This small patchset is about avoid verifier bug warning when conditional jumps on same register when the register holds a scalar with range. v4: - make code better. (Alexei) v3: https://lore.kernel.org/bpf/[email protected]/ - Enhance is_scalar_branch_taken() to handle scalar case. (Eduard) - Update the selftest to cover all conditional jump opcodes. (Eduard) v2: https://lore.kernel.org/bpf/[email protected]/ - Enhance is_branch_taken() and is_scalar_branch_taken() to handle branch direction computation for same register. (Eduard and Alexei) - Update the selftest. v1: https://lore.kernel.org/bpf/[email protected]/ --- ==================== Link: https://patch.msgid.link/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents 5dae745 + 9f32bfe commit 11369e6

File tree

2 files changed

+185
-0
lines changed

2 files changed

+185
-0
lines changed

‎kernel/bpf/verifier.c‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15993,6 +15993,30 @@ static int is_scalar_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_sta
1599315993
s64 smin2 = is_jmp32 ? (s64)reg2->s32_min_value : reg2->smin_value;
1599415994
s64 smax2 = is_jmp32 ? (s64)reg2->s32_max_value : reg2->smax_value;
1599515995

15996+
if (reg1 == reg2) {
15997+
switch (opcode) {
15998+
case BPF_JGE:
15999+
case BPF_JLE:
16000+
case BPF_JSGE:
16001+
case BPF_JSLE:
16002+
case BPF_JEQ:
16003+
return 1;
16004+
case BPF_JGT:
16005+
case BPF_JLT:
16006+
case BPF_JSGT:
16007+
case BPF_JSLT:
16008+
case BPF_JNE:
16009+
return 0;
16010+
case BPF_JSET:
16011+
if (tnum_is_const(t1))
16012+
return t1.value != 0;
16013+
else
16014+
return (smin1 <= 0 && smax1 >= 0) ? -1 : 1;
16015+
default:
16016+
return -1;
16017+
}
16018+
}
16019+
1599616020
switch (opcode) {
1599716021
case BPF_JEQ:
1599816022
/* constants, umin/umax and smin/smax checks would be
@@ -16439,6 +16463,13 @@ static int reg_set_min_max(struct bpf_verifier_env *env,
1643916463
if (false_reg1->type != SCALAR_VALUE || false_reg2->type != SCALAR_VALUE)
1644016464
return 0;
1644116465

16466+
/* We compute branch direction for same SCALAR_VALUE registers in
16467+
* is_scalar_branch_taken(). For unknown branch directions (e.g., BPF_JSET)
16468+
* on the same registers, we don't need to adjust the min/max values.
16469+
*/
16470+
if (false_reg1 == false_reg2)
16471+
return 0;
16472+
1644216473
/* fallthrough (FALSE) branch */
1644316474
regs_refine_cond_op(false_reg1, false_reg2, rev_opcode(opcode), is_jmp32);
1644416475
reg_bounds_sync(false_reg1);

‎tools/testing/selftests/bpf/progs/verifier_bounds.c‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1709,4 +1709,158 @@ __naked void jeq_disagreeing_tnums(void *ctx)
17091709
: __clobber_all);
17101710
}
17111711

1712+
SEC("socket")
1713+
__description("conditional jump on same register, branch taken")
1714+
__not_msg("20: (b7) r0 = 1 {{.*}} R0=1")
1715+
__success __log_level(2)
1716+
__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
1717+
__naked void condition_jump_on_same_register(void *ctx)
1718+
{
1719+
asm volatile(" \
1720+
call %[bpf_get_prandom_u32]; \
1721+
w8 = 0x80000000; \
1722+
r0 &= r8; \
1723+
if r0 == r0 goto +1; \
1724+
goto l1_%=; \
1725+
if r0 >= r0 goto +1; \
1726+
goto l1_%=; \
1727+
if r0 s>= r0 goto +1; \
1728+
goto l1_%=; \
1729+
if r0 <= r0 goto +1; \
1730+
goto l1_%=; \
1731+
if r0 s<= r0 goto +1; \
1732+
goto l1_%=; \
1733+
if r0 != r0 goto l1_%=; \
1734+
if r0 > r0 goto l1_%=; \
1735+
if r0 s> r0 goto l1_%=; \
1736+
if r0 < r0 goto l1_%=; \
1737+
if r0 s< r0 goto l1_%=; \
1738+
l0_%=: r0 = 0; \
1739+
exit; \
1740+
l1_%=: r0 = 1; \
1741+
exit; \
1742+
" :
1743+
: __imm(bpf_get_prandom_u32)
1744+
: __clobber_all);
1745+
}
1746+
1747+
SEC("socket")
1748+
__description("jset on same register, constant value branch taken")
1749+
__not_msg("7: (b7) r0 = 1 {{.*}} R0=1")
1750+
__success __log_level(2)
1751+
__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
1752+
__naked void jset_on_same_register_1(void *ctx)
1753+
{
1754+
asm volatile(" \
1755+
r0 = 0; \
1756+
if r0 & r0 goto l1_%=; \
1757+
r0 = 1; \
1758+
if r0 & r0 goto +1; \
1759+
goto l1_%=; \
1760+
l0_%=: r0 = 0; \
1761+
exit; \
1762+
l1_%=: r0 = 1; \
1763+
exit; \
1764+
" :
1765+
: __imm(bpf_get_prandom_u32)
1766+
: __clobber_all);
1767+
}
1768+
1769+
SEC("socket")
1770+
__description("jset on same register, scalar value branch taken")
1771+
__not_msg("12: (b7) r0 = 1 {{.*}} R0=1")
1772+
__success __log_level(2)
1773+
__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
1774+
__naked void jset_on_same_register_2(void *ctx)
1775+
{
1776+
asm volatile(" \
1777+
/* range [1;2] */ \
1778+
call %[bpf_get_prandom_u32]; \
1779+
r0 &= 0x1; \
1780+
r0 += 1; \
1781+
if r0 & r0 goto +1; \
1782+
goto l1_%=; \
1783+
/* range [-2;-1] */ \
1784+
call %[bpf_get_prandom_u32]; \
1785+
r0 &= 0x1; \
1786+
r0 -= 2; \
1787+
if r0 & r0 goto +1; \
1788+
goto l1_%=; \
1789+
l0_%=: r0 = 0; \
1790+
exit; \
1791+
l1_%=: r0 = 1; \
1792+
exit; \
1793+
" :
1794+
: __imm(bpf_get_prandom_u32)
1795+
: __clobber_all);
1796+
}
1797+
1798+
SEC("socket")
1799+
__description("jset on same register, scalar value unknown branch 1")
1800+
__msg("3: (b7) r0 = 0 {{.*}} R0=0")
1801+
__msg("5: (b7) r0 = 1 {{.*}} R0=1")
1802+
__success __log_level(2)
1803+
__flag(BPF_F_TEST_REG_INVARIANTS)
1804+
__naked void jset_on_same_register_3(void *ctx)
1805+
{
1806+
asm volatile(" \
1807+
/* range [0;1] */ \
1808+
call %[bpf_get_prandom_u32]; \
1809+
r0 &= 0x1; \
1810+
if r0 & r0 goto l1_%=; \
1811+
l0_%=: r0 = 0; \
1812+
exit; \
1813+
l1_%=: r0 = 1; \
1814+
exit; \
1815+
" :
1816+
: __imm(bpf_get_prandom_u32)
1817+
: __clobber_all);
1818+
}
1819+
1820+
SEC("socket")
1821+
__description("jset on same register, scalar value unknown branch 2")
1822+
__msg("4: (b7) r0 = 0 {{.*}} R0=0")
1823+
__msg("6: (b7) r0 = 1 {{.*}} R0=1")
1824+
__success __log_level(2)
1825+
__flag(BPF_F_TEST_REG_INVARIANTS)
1826+
__naked void jset_on_same_register_4(void *ctx)
1827+
{
1828+
asm volatile(" \
1829+
/* range [-1;0] */ \
1830+
call %[bpf_get_prandom_u32]; \
1831+
r0 &= 0x1; \
1832+
r0 -= 1; \
1833+
if r0 & r0 goto l1_%=; \
1834+
l0_%=: r0 = 0; \
1835+
exit; \
1836+
l1_%=: r0 = 1; \
1837+
exit; \
1838+
" :
1839+
: __imm(bpf_get_prandom_u32)
1840+
: __clobber_all);
1841+
}
1842+
1843+
SEC("socket")
1844+
__description("jset on same register, scalar value unknown branch 3")
1845+
__msg("4: (b7) r0 = 0 {{.*}} R0=0")
1846+
__msg("6: (b7) r0 = 1 {{.*}} R0=1")
1847+
__success __log_level(2)
1848+
__flag(BPF_F_TEST_REG_INVARIANTS)
1849+
__naked void jset_on_same_register_5(void *ctx)
1850+
{
1851+
asm volatile(" \
1852+
/* range [-1;1] */ \
1853+
call %[bpf_get_prandom_u32]; \
1854+
r0 &= 0x2; \
1855+
r0 -= 1; \
1856+
if r0 & r0 goto l1_%=; \
1857+
l0_%=: r0 = 0; \
1858+
exit; \
1859+
l1_%=: r0 = 1; \
1860+
exit; \
1861+
" :
1862+
: __imm(bpf_get_prandom_u32)
1863+
: __clobber_all);
1864+
}
1865+
17121866
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)