Skip to content

Commit e5ffcc9

Browse files
author
Alexei Starovoitov
committed
Merge branch 'subreg-bounds'
John Fastabend says: ==================== This series adds ALU32 signed and unsigned min/max bounds. The origins of this work is to fix do_refine_retval_range() which before this series clamps the return value bounds to [0, max]. However, this is not correct because its possible these functions may return negative errors so the correct bound is [*MIN, max]. Where *MIN is the signed and unsigned min values U64_MIN and S64_MIN. And 'max' here is the max positive value returned by this routine. Patch 1 changes the do_refine_retval_range() to return the correct bounds but this breaks existing programs that were depending on the old incorrect bound. To repair these old programs we add ALU32 bounds to properly track the return values from these helpers. The ALU32 bounds are needed because clang realizes these helepers return 'int' type and will use jmp32 ops with the return value. With current state of things this does little to help 64bit bounds and with patch 1 applied will cause many programs to fail verifier pass. See patch 5 for trace details on how this happens. Patch 2 does the ALU32 addition it adds the new bounds and populates them through the verifier. Design note, initially a var32 was added but as pointed out by Alexei and Edward it is not strictly needed so it was removed here. This worked out nicely. Patch 3 notes that the refine return value can now also bound the 32-bit subregister allowing better bouinds tracking in these cases. Patches 4 adds a C test case to test_progs which will cause the verifier to fail if new 32bit and do_refine_retval_range() is incorrect. Patches 5 and 6 fix test cases that broke after refining the return values from helpers. I attempted to be explicit about each failure and why we need the change. See patches for details. Patch 7 adds some bounds check tests to ensure bounds checking when mixing alu32, alu64 and jmp32 ops together. Thanks to Alexei, Edward, and Daniel for initial feedback it helped clean this up a lot. v2: - rebased to bpf-next - fixed tnum equals optimization for combining 32->64bits - updated patch to fix verifier test correctly - updated refine_retval_range to set both s32_*_value and s*_value we need both to get better bounds tracking ==================== Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents 4edf16b + 41f70fe commit e5ffcc9

File tree

9 files changed

+959
-301
lines changed

9 files changed

+959
-301
lines changed

include/linux/bpf_verifier.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ struct bpf_reg_state {
123123
s64 smax_value; /* maximum possible (s64)value */
124124
u64 umin_value; /* minimum possible (u64)value */
125125
u64 umax_value; /* maximum possible (u64)value */
126+
s32 s32_min_value; /* minimum possible (s32)value */
127+
s32 s32_max_value; /* maximum possible (s32)value */
128+
u32 u32_min_value; /* minimum possible (u32)value */
129+
u32 u32_max_value; /* maximum possible (u32)value */
126130
/* parentage chain for liveness checking */
127131
struct bpf_reg_state *parent;
128132
/* Inside the callee two registers can be both PTR_TO_STACK like

include/linux/limits.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#define S16_MAX ((s16)(U16_MAX >> 1))
2828
#define S16_MIN ((s16)(-S16_MAX - 1))
2929
#define U32_MAX ((u32)~0U)
30+
#define U32_MIN ((u32)0)
3031
#define S32_MAX ((s32)(U32_MAX >> 1))
3132
#define S32_MIN ((s32)(-S32_MAX - 1))
3233
#define U64_MAX ((u64)~0ULL)

include/linux/tnum.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,16 @@ int tnum_strn(char *str, size_t size, struct tnum a);
8686
/* Format a tnum as tristate binary expansion */
8787
int tnum_sbin(char *str, size_t size, struct tnum a);
8888

89+
/* Returns the 32-bit subreg */
90+
struct tnum tnum_subreg(struct tnum a);
91+
/* Returns the tnum with the lower 32-bit subreg cleared */
92+
struct tnum tnum_clear_subreg(struct tnum a);
93+
/* Returns the tnum with the lower 32-bit subreg set to value */
94+
struct tnum tnum_const_subreg(struct tnum a, u32 value);
95+
/* Returns true if 32-bit subreg @a is a known constant*/
96+
static inline bool tnum_subreg_is_const(struct tnum a)
97+
{
98+
return !(tnum_subreg(a)).mask;
99+
}
100+
89101
#endif /* _LINUX_TNUM_H */

kernel/bpf/tnum.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,3 +194,18 @@ int tnum_sbin(char *str, size_t size, struct tnum a)
194194
str[min(size - 1, (size_t)64)] = 0;
195195
return 64;
196196
}
197+
198+
struct tnum tnum_subreg(struct tnum a)
199+
{
200+
return tnum_cast(a, 4);
201+
}
202+
203+
struct tnum tnum_clear_subreg(struct tnum a)
204+
{
205+
return tnum_lshift(tnum_rshift(a, 32), 32);
206+
}
207+
208+
struct tnum tnum_const_subreg(struct tnum a, u32 value)
209+
{
210+
return tnum_or(tnum_clear_subreg(a), tnum_const(value));
211+
}

0 commit comments

Comments
 (0)