Re: [DO_NOT_APPLY 1/2] integrate xdp-forwarder into net-vm

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

Yureka <yuka@yuka.dev> writes:

>>> +
>>> +	/* Byte-count bounds check; check if current pointer + size of header
>>> +	 * is after data_end.
>>> +	 */
>>> +	if ((void *) (eth + 1) > data_end)
>>> +		return -1;
>> This is checking that there's more data after the header, right?  Is
>> that something it's important for us to check?
>
> The intent is to check that the entire eth hdr, which we casted a 
> pointer to, is within the data (length) of the packet before we 
> de-reference the pointer. So essentially, skipping packets which do not 
> have a full ethernet header, instead of reading from addresses which we 
> are not supposed to read from.
>
> When loading the XDP program, it is tested against an empty or very 
> small packet, and if it tries to access memory outside of the packet 
> bounds, it will refuse to load. So the BPF/XDP system ensures that these 
> kinds of packets are handled properly.

Doesn't using > instead of >= check that the entire eth hdr **plus one
byte** is within the packet, though?  i.e. wouldn't this check fail if
the data consisted entirely of an ethernet header?  Is that the right
thing to do?  (Sorry if my maths is just wrong.)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]