pping: Add check to satisfy verifier

Add a check that to ensure verifier that opt_size is positive in case
its been read in from stack. Also enable (uncomment) the flow-state
cleanup from the XDP program as the added check avoids verifier
rejection.

Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
This commit is contained in:
Simon Sundberg
2021-03-15 18:23:23 +01:00
parent 2b64355b2e
commit e04284a3ee
2 changed files with 15 additions and 7 deletions

View File

@@ -77,7 +77,7 @@ static int parse_tcp_ts(struct tcphdr *tcph, void *data_end, __u32 *tsval,
if (tcph + 1 > data_end || len <= sizeof(struct tcphdr))
return -1;
#pragma unroll //temporary solution until we can identify why the non-unrolled loop gets stuck in an infinite loop
for (i = 0; i < MAX_TCP_OPTIONS; i++) {
if (pos + 1 > opt_end || pos + 1 > data_end)
return -1;
@@ -107,8 +107,17 @@ static int parse_tcp_ts(struct tcphdr *tcph, void *data_end, __u32 *tsval,
}
// Some other TCP option - advance option-length bytes
if (opt_size < 0 || opt_size > 34) // Try to convince verifier that opt-size can't be something crazy - leads to program being too large instead...
return -1;
/*
* The &= is to make sure that verifier knows opt_size must
* be positive, as verifier may not remeber u8 constraint if
* it fetches variable from stack, see ex
* https://blog.path.net/ebpf-xdp-and-network-security/.
* Use 0x3f instead of 0xff to avoid compiler optimizing
* away the check, and no option should be able to have a
* size of above 63 anyways (maximum TCP header size is 64
* bytes, and no more than 64-20=44 of them can by options).
*/
opt_size &= 0x3f;
pos += opt_size;
}
return -1;

View File

@@ -32,10 +32,9 @@ int pping_ingress(struct xdp_md *ctx)
if (parse_packet_identifier(&pctx, &p_id, &flow_closing) < 0)
goto out;
/* // Delete flow, but allow final attempt at RTT calculation */
/* if (flow_closing) // For some reason verifier is very unhappy about this */
/* bpf_map_delete_elem(&flow_state, &p_id.flow); */
// Delete flow, but allow final attempt at RTT calculation
if (flow_closing)
bpf_map_delete_elem(&flow_state, &p_id.flow);
p_ts = bpf_map_lookup_elem(&ts_start, &p_id);
if (!p_ts)