On 1/11/22 9:20 AM, Dan Carpenter wrote:
Hello Daniel Borkmann,
[+John]
The patch e572ff80f05c: "bpf: Make 32->64 bounds propagation slightly
more robust" from Dec 15, 2021, leads to the following Smatch static
checker warning:
kernel/bpf/verifier.c:1412 __reg32_bound_s64()
warn: always true condition '(a <= (((~0) >> 1))) => (s32min-s32max <= s32max)'
kernel/bpf/verifier.c
1410 static bool __reg32_bound_s64(s32 a)
1411 {
1412 return a >= 0 && a <= S32_MAX;
Obviously an s32 is going to be <= S32_MAX
It's aligned with similar helpers such as __reg64_bound_u32() / __reg64_bound_s32() and
when discussing we went for leaving this explicitly documented in here (aside being true).
1413 }
1414
1415 static void __reg_assign_32_into_64(struct bpf_reg_state *reg)
1416 {
1417 reg->umin_value = reg->u32_min_value;
1418 reg->umax_value = reg->u32_max_value;
1419
1420 /* Attempt to pull 32-bit signed bounds into 64-bit bounds but must
1421 * be positive otherwise set to worse case bounds and refine later
1422 * from tnum.
1423 */
1424 if (__reg32_bound_s64(reg->s32_min_value) &&
1425 __reg32_bound_s64(reg->s32_max_value)) {
1426 reg->smin_value = reg->s32_min_value;
1427 reg->smax_value = reg->s32_max_value;
1428 } else {
1429 reg->smin_value = 0;
1430 reg->smax_value = U32_MAX;
Should this be S32_MAX instead of U32_MAX?
U32_MAX is correct here.
1431 }
1432 }
regards,
dan carpenter
