On Sat, Dec 12, 2020 at 5:01 AM Vasily Averin <v...@virtuozzo.com> wrote: > > On 12/11/20 6:37 PM, Vasily Averin wrote: > > It seems for me the similar problem can happen in __skb_trim_rcsum(). > > Also I doubt that that skb_checksum_start_offset(skb) checks in > > __skb_postpull_rcsum() and skb_csum_unnecessary() are correct, > > becasue they do not guarantee that skb have correct CHECKSUM_PARTIAL. > > Could somebody confirm it? > > I've rechecked the code and I think now that other places are not affected, > i.e. skb_push_rcsum() only should be patched.
Thanks for investigating this. So tun was able to insert a packet with csum_start + csum_off + 2 beyond the packet after trimming, using virtio_net_hdr.csum_... Any packet with an offset beyond the end of the packet is bogus really. No need to try to accept it by downgrading to CHECKSUM_NONE. It is not ideal to have to add branches in the common path for these obscure bad packets from virtio/tuntap/af_packet. We try to avoid that with more strict validation at the source in virtio_net_hdr_to_skb. Evidently syzbot again found a way past again. If this is a packet with gso_type and checksum offload, we know the accepted protocols and can validate the offset. If gso_type is none, however, no such assumptions can be made. All we could do is try to dissect and if a known protocol and valid th_off, compare that to the checksum fields passed by userspace. So that path is certainly more complex than your fix, which works as well.
