Skip to content

Commit 977bc14

Browse files
eddyz87Alexei Starovoitov
authored andcommitted
selftests/bpf: track tcp payload offset as scalar in xdp_synproxy
This change prepares syncookie_{tc,xdp} for update in callbakcs verification logic. To allow bpf_loop() verification converge when multiple callback itreations are considered: - track offset inside TCP payload explicitly, not as a part of the pointer; - make sure that offset does not exceed MAX_PACKET_OFF enforced by verifier; - make sure that offset is tracked as unbound scalar between iterations, otherwise verifier won't be able infer that bpf_loop callback reaches identical states. Acked-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> Link: https://lore.kernel.org/r/20231121020701.26440-2-eddyz87@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent fcb905d commit 977bc14

File tree

1 file changed

+52
-32
lines changed

1 file changed

+52
-32
lines changed

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

Lines changed: 52 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
#define DEFAULT_TTL 64
5454
#define MAX_ALLOWED_PORTS 8
5555

56+
#define MAX_PACKET_OFF 0xffff
57+
5658
#define swap(a, b) \
5759
do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
5860

@@ -183,63 +185,76 @@ static __always_inline __u32 tcp_clock_ms(void)
183185
}
184186

185187
struct tcpopt_context {
186-
__u8 *ptr;
187-
__u8 *end;
188+
void *data;
188189
void *data_end;
189190
__be32 *tsecr;
190191
__u8 wscale;
191192
bool option_timestamp;
192193
bool option_sack;
194+
__u32 off;
193195
};
194196

195-
static int tscookie_tcpopt_parse(struct tcpopt_context *ctx)
197+
static __always_inline u8 *next(struct tcpopt_context *ctx, __u32 sz)
196198
{
197-
__u8 opcode, opsize;
199+
__u64 off = ctx->off;
200+
__u8 *data;
198201

199-
if (ctx->ptr >= ctx->end)
200-
return 1;
201-
if (ctx->ptr >= ctx->data_end)
202-
return 1;
202+
/* Verifier forbids access to packet when offset exceeds MAX_PACKET_OFF */
203+
if (off > MAX_PACKET_OFF - sz)
204+
return NULL;
203205

204-
opcode = ctx->ptr[0];
206+
data = ctx->data + off;
207+
barrier_var(data);
208+
if (data + sz >= ctx->data_end)
209+
return NULL;
205210

206-
if (opcode == TCPOPT_EOL)
207-
return 1;
208-
if (opcode == TCPOPT_NOP) {
209-
++ctx->ptr;
210-
return 0;
211-
}
211+
ctx->off += sz;
212+
return data;
213+
}
212214

213-
if (ctx->ptr + 1 >= ctx->end)
214-
return 1;
215-
if (ctx->ptr + 1 >= ctx->data_end)
215+
static int tscookie_tcpopt_parse(struct tcpopt_context *ctx)
216+
{
217+
__u8 *opcode, *opsize, *wscale, *tsecr;
218+
__u32 off = ctx->off;
219+
220+
opcode = next(ctx, 1);
221+
if (!opcode)
216222
return 1;
217-
opsize = ctx->ptr[1];
218-
if (opsize < 2)
223+
224+
if (*opcode == TCPOPT_EOL)
219225
return 1;
226+
if (*opcode == TCPOPT_NOP)
227+
return 0;
220228

221-
if (ctx->ptr + opsize > ctx->end)
229+
opsize = next(ctx, 1);
230+
if (!opsize || *opsize < 2)
222231
return 1;
223232

224-
switch (opcode) {
233+
switch (*opcode) {
225234
case TCPOPT_WINDOW:
226-
if (opsize == TCPOLEN_WINDOW && ctx->ptr + TCPOLEN_WINDOW <= ctx->data_end)
227-
ctx->wscale = ctx->ptr[2] < TCP_MAX_WSCALE ? ctx->ptr[2] : TCP_MAX_WSCALE;
235+
wscale = next(ctx, 1);
236+
if (!wscale)
237+
return 1;
238+
if (*opsize == TCPOLEN_WINDOW)
239+
ctx->wscale = *wscale < TCP_MAX_WSCALE ? *wscale : TCP_MAX_WSCALE;
228240
break;
229241
case TCPOPT_TIMESTAMP:
230-
if (opsize == TCPOLEN_TIMESTAMP && ctx->ptr + TCPOLEN_TIMESTAMP <= ctx->data_end) {
242+
tsecr = next(ctx, 4);
243+
if (!tsecr)
244+
return 1;
245+
if (*opsize == TCPOLEN_TIMESTAMP) {
231246
ctx->option_timestamp = true;
232247
/* Client's tsval becomes our tsecr. */
233-
*ctx->tsecr = get_unaligned((__be32 *)(ctx->ptr + 2));
248+
*ctx->tsecr = get_unaligned((__be32 *)tsecr);
234249
}
235250
break;
236251
case TCPOPT_SACK_PERM:
237-
if (opsize == TCPOLEN_SACK_PERM)
252+
if (*opsize == TCPOLEN_SACK_PERM)
238253
ctx->option_sack = true;
239254
break;
240255
}
241256

242-
ctx->ptr += opsize;
257+
ctx->off = off + *opsize;
243258

244259
return 0;
245260
}
@@ -256,16 +271,21 @@ static int tscookie_tcpopt_parse_batch(__u32 index, void *context)
256271

257272
static __always_inline bool tscookie_init(struct tcphdr *tcp_header,
258273
__u16 tcp_len, __be32 *tsval,
259-
__be32 *tsecr, void *data_end)
274+
__be32 *tsecr, void *data, void *data_end)
260275
{
261276
struct tcpopt_context loop_ctx = {
262-
.ptr = (__u8 *)(tcp_header + 1),
263-
.end = (__u8 *)tcp_header + tcp_len,
277+
.data = data,
264278
.data_end = data_end,
265279
.tsecr = tsecr,
266280
.wscale = TS_OPT_WSCALE_MASK,
267281
.option_timestamp = false,
268282
.option_sack = false,
283+
/* Note: currently verifier would track .off as unbound scalar.
284+
* In case if verifier would at some point get smarter and
285+
* compute bounded value for this var, beware that it might
286+
* hinder bpf_loop() convergence validation.
287+
*/
288+
.off = (__u8 *)(tcp_header + 1) - (__u8 *)data,
269289
};
270290
u32 cookie;
271291

@@ -635,7 +655,7 @@ static __always_inline int syncookie_handle_syn(struct header_pointers *hdr,
635655
cookie = (__u32)value;
636656

637657
if (tscookie_init((void *)hdr->tcp, hdr->tcp_len,
638-
&tsopt_buf[0], &tsopt_buf[1], data_end))
658+
&tsopt_buf[0], &tsopt_buf[1], data, data_end))
639659
tsopt = tsopt_buf;
640660

641661
/* Check that there is enough space for a SYNACK. It also covers

0 commit comments

Comments
 (0)