On 09/11/21 17:13, Alexei Starovoitov wrote:
On Tue, Nov 9, 2021 at 5:47 AM Federico Parola
<federico.parola@xxxxxxxxx> wrote:
Thanks for your answer.
Please do not top post and don't drop mailing list.
If I perform something like:
*(__u16 *)data &= 0xefff;
data += *(__u16 *)data;
To limit the max value of my offset the program is accepted, is this
what you mean with "clamping"?
kinda. I don't think you meant to mangle the packet in the above.
So packet pointers are stored on 16 bits? And every time we add an
offset we must guarantee not to overflow these size?
no. the pointers are 64-bit, but there could be additional alu ops
on them. So MAX_PACKET_OFF was picked as the practical limit.
Do you have a real life use case where you need to add full u16?
Definitely not, both my use cases can be solved limiting the size of the
offset with a mask (I suppose a comparison with an upper limit works as
well).
Thank you.
On 09/11/21 06:29, Alexei Starovoitov wrote:
On Mon, Nov 8, 2021 at 6:04 AM Federico Parola
<federico.parola@xxxxxxxxx> wrote:
Dear all,
I found out that every time an offset stored in a 2 (or more) bytes
variable is added to a packet pointer subsequent checks against packet
boundaries become ineffective.
Here is a toy example to test the problem (it doesn't do anything useful):
int test(struct __sk_buff *ctx) {
void *data = (void *)(long)ctx->data;
void *data_end = (void *)(long)ctx->data_end;
/* Skipping an amount of bytes stored in __u8 works */
if (data + sizeof(__u8) > data_end)
return TC_ACT_OK;
bpf_trace_printk("Skipping %d bytes", *(__u8 *)data);
data += *(__u8 *)data;
/* Skipping an amount of bytes stored in __u16 works but... */
if (data + sizeof(__u16) > data_end)
return TC_ACT_OK;
bpf_trace_printk("Skipping %d bytes", *(__u16 *)data);
data += *(__u16 *)data;
/* ...this check is not effective and packet access is rejected */
if (data + sizeof(__u8) > data_end)
return TC_ACT_OK;
bpf_trace_printk("Next byte is %x", *(__u8 *)data);
return TC_ACT_OK;
}
My practical use case would be skipping variable-size TLS header
extensions until I reach the desired one (the length of these options is
2 bytes long).
Another use case can be found here:
https://lists.iovisor.org/g/iovisor-dev/topic/access_packet_payload_in_tc/86442134
After I use the bpf_skb_pull_data() I would like to directly jump to the
part of packet I was working on and avoid re-parsing everything from
scratch, however if I save the offset in a 2 bytes variable and then add
it to the packet pointer I'm no longer able to access it (if the offset
is stored in a 1 byte var everything works).
Is this a verifier bug?
It's because of:
if (dst_reg->umax_value > MAX_PACKET_OFF ||
dst_reg->umax_value + dst_reg->off > MAX_PACKET_OFF)
/* Risk of overflow. For instance, ptr + (1<<63) may be less
* than pkt_end, but that's because it's also less than pkt.
*/
return;
by adding u16 scalar the offset becomes bigger than MAX_PACKET_OFF.
Could you try clamping the value before 'data += ' ?
