Jenny Guanni Qu says: ==================== bpf: Fix abs(INT_MIN) undefined behavior in interpreter sdiv/smod The BPF interpreter's signed 32-bit division and modulo handlers use abs() on s32 operands, which is undefined for S32_MIN. This causes the interpreter to compute wrong results, creating a mismatch with the verifier's range tracking. For example, INT_MIN / 2 returns 0x40000000 instead of the correct 0xC0000000. The verifier tracks the correct range, so a crafted BPF program can exploit the mismatch for out-of-bounds map value access (confirmed by KASAN). Patch 1 introduces abs_s32() which handles S32_MIN correctly and replaces all 8 abs((s32)...) call sites. s32 is the only affected case -- the s64 handlers do not use abs(). Patch 2 adds selftests covering sdiv32 and smod32 with INT_MIN dividend to prevent regression. Changes since v4: - Renamed __safe_abs32() to abs_s32() and dropped inline keyword per Alexei Starovoitov's feedback Changes since v3: - Fixed stray blank line deletion in the file header - Improved comment per Yonghong Song's suggestion - Added JIT vs interpreter context to selftest commit message Changes since v2: - Simplified to use -(u32)x per Mykyta Yatsenko's suggestion Changes since v1: - Moved helper above kerneldoc comment block to fix build warnings ====================
Unified · Split
| @@ -1757,6 +1757,12 @@ bool bpf_opcode_in_insntable(u8 code) · 1756 unmodified lines | ||
| 1757 | 1757 | } |
| 1758 | 1758 | |
| 1759 | 1759 | #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 | + | |
| 1760 | 1766 | /** |
| 1761 | 1767 | * ___bpf_prog_run - run eBPF program on a given context |
| 1762 | 1768 | * @regs: is the array of MAX_BPF_EXT_REG eBPF pseudo-registers |
| @@ -1921,8 +1927,8 @@ select_insn: · 158 unmodified lines | ||
| 1921 | 1927 | DST = do_div(AX, (u32) SRC); |
| 1922 | 1928 | break; |
| 1923 | 1929 | case 1: |
| 1924 | − AX = abs((s32)DST); | |
| 1925 | − AX = do_div(AX, abs((s32)SRC)); | |
| 1930 | + AX = abs_s32((s32)DST); | |
| 1931 | + AX = do_div(AX, abs_s32((s32)SRC)); | |
| 1926 | 1932 | if ((s32)DST < 0) |
| 1927 | 1933 | DST = (u32)-AX; |
| 1928 | 1934 | else |
| @@ -1949,8 +1955,8 @@ select_insn: · 20 unmodified lines | ||
| 1949 | 1955 | DST = do_div(AX, (u32) IMM); |
| 1950 | 1956 | break; |
| 1951 | 1957 | case 1: |
| 1952 | − AX = abs((s32)DST); | |
| 1953 | − AX = do_div(AX, abs((s32)IMM)); | |
| 1958 | + AX = abs_s32((s32)DST); | |
| 1959 | + AX = do_div(AX, abs_s32((s32)IMM)); | |
| 1954 | 1960 | if ((s32)DST < 0) |
| 1955 | 1961 | DST = (u32)-AX; |
| 1956 | 1962 | else |
| @@ -1976,8 +1982,8 @@ select_insn: · 19 unmodified lines | ||
| 1976 | 1982 | DST = (u32) AX; |
| 1977 | 1983 | break; |
| 1978 | 1984 | case 1: |
| 1979 | − AX = abs((s32)DST); | |
| 1980 | − do_div(AX, abs((s32)SRC)); | |
| 1985 | + AX = abs_s32((s32)DST); | |
| 1986 | + do_div(AX, abs_s32((s32)SRC)); | |
| 1981 | 1987 | if (((s32)DST < 0) == ((s32)SRC < 0)) |
| 1982 | 1988 | DST = (u32)AX; |
| 1983 | 1989 | else |
| @@ -2003,8 +2009,8 @@ select_insn: · 19 unmodified lines | ||
| 2003 | 2009 | DST = (u32) AX; |
| 2004 | 2010 | break; |
| 2005 | 2011 | case 1: |
| 2006 | − AX = abs((s32)DST); | |
| 2007 | − do_div(AX, abs((s32)IMM)); | |
| 2012 | + AX = abs_s32((s32)DST); | |
| 2013 | + do_div(AX, abs_s32((s32)IMM)); | |
| 2008 | 2014 | if (((s32)DST < 0) == ((s32)IMM < 0)) |
| 2009 | 2015 | DST = (u32)AX; |
| 2010 | 2016 | else |
| @@ -1209,6 +1209,64 @@ __naked void smod32_ri_divisor_neg_1(void) · 1208 unmodified lines | ||
| 1209 | 1209 | : __clobber_all); |
| 1210 | 1210 | } |
| 1211 | 1211 | |
| 1212 | +SEC("socket") | |
| 1213 | +__description("SDIV32, INT_MIN divided by 2, imm") | |
| 1214 | +__success __success_unpriv __retval(-1073741824) | |
| 1215 | +__naked void sdiv32_int_min_div_2_imm(void) | |
| 1216 | +{ | |
| 1217 | + asm volatile (" \ | |
| 1218 | + w0 = %[int_min]; \ | |
| 1219 | + w0 s/= 2; \ | |
| 1220 | + exit; \ | |
| 1221 | +" : | |
| 1222 | + : __imm_const(int_min, INT_MIN) | |
| 1223 | + : __clobber_all); | |
| 1224 | +} | |
| 1225 | + | |
| 1226 | +SEC("socket") | |
| 1227 | +__description("SDIV32, INT_MIN divided by 2, reg") | |
| 1228 | +__success __success_unpriv __retval(-1073741824) | |
| 1229 | +__naked void sdiv32_int_min_div_2_reg(void) | |
| 1230 | +{ | |
| 1231 | + asm volatile (" \ | |
| 1232 | + w0 = %[int_min]; \ | |
| 1233 | + w1 = 2; \ | |
| 1234 | + w0 s/= w1; \ | |
| 1235 | + exit; \ | |
| 1236 | +" : | |
| 1237 | + : __imm_const(int_min, INT_MIN) | |
| 1238 | + : __clobber_all); | |
| 1239 | +} | |
| 1240 | + | |
| 1241 | +SEC("socket") | |
| 1242 | +__description("SMOD32, INT_MIN modulo 2, imm") | |
| 1243 | +__success __success_unpriv __retval(0) | |
| 1244 | +__naked void smod32_int_min_mod_2_imm(void) | |
| 1245 | +{ | |
| 1246 | + asm volatile (" \ | |
| 1247 | + w0 = %[int_min]; \ | |
| 1248 | + w0 s%%= 2; \ | |
| 1249 | + exit; \ | |
| 1250 | +" : | |
| 1251 | + : __imm_const(int_min, INT_MIN) | |
| 1252 | + : __clobber_all); | |
| 1253 | +} | |
| 1254 | + | |
| 1255 | +SEC("socket") | |
| 1256 | +__description("SMOD32, INT_MIN modulo -2, imm") | |
| 1257 | +__success __success_unpriv __retval(0) | |
| 1258 | +__naked void smod32_int_min_mod_neg2_imm(void) | |
| 1259 | +{ | |
| 1260 | + asm volatile (" \ | |
| 1261 | + w0 = %[int_min]; \ | |
| 1262 | + w0 s%%= -2; \ | |
| 1263 | + exit; \ | |
| 1264 | +" : | |
| 1265 | + : __imm_const(int_min, INT_MIN) | |
| 1266 | + : __clobber_all); | |
| 1267 | +} | |
| 1268 | + | |
| 1269 | + | |
| 1212 | 1270 | #else |
| 1213 | 1271 | |
| 1214 | 1272 | SEC("socket") |