Michael S. Tsirkin wrote: > 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.
I also have no concerns with the commit itself. It would become an issue only if tap would support it and trust hdr_len unconditionally. Acked-by: Willem de Bruijn <will...@google.com> > 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@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
