Re: [patch net-next v2] net: virtio_net: implement exact header length guest feature

On Wed, Feb 22, 2023 at 10:14:21AM -0500, Willem de Bruijn wrote:

Either including the link that Michael shared or quoting the relevant
part verbatim in the commit message would help, thanks.

Thinking it over, my main concern is that the prescriptive section in
the spec does not state what to do when the value is clearly garbage,
as we have seen with syzkaller.

Having to sanitize input, by dropping if < ETH_HLEN or > length, to
me means that the device cannot trust the field, as the spec says it
should. 

Right. I think the implication is that if device detects and illegal
value it's OK for it to just drop the packet or reset or enter
a broken mode until reset.

By contrast without the feature bit the header size can be
used as a hint e.g. to size allocations but you must
recover if it's incorrect.

And yes tap seems to break if you make it too small or if you make
it huge so it does not really follow the spec in this regard.

Setting the flag will not fix tap because we can't really
affort breaking all drivers who don't set it. But it will
prepare the ground for when tens of years from now we
actually look back and say all drivers set it, no problem.

So that's a good reason to ack this patch.

However if someone is worried about this then fixing tap
so it recovers from incorrect header length without
packet loss is a good idea.

Sanitization is harder in the kernel, because it has to support all
kinds of link layers, including variable length.

Perhaps that's a discussion for the spec rather than this commit. But
it's a point to clarify as we add support to the code.

-- 
MST