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 _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
