On Tue, Dec 4, 2018 at 11:33 AM Saeed Mahameed <sae...@dev.mellanox.co.il> wrote: > > On Sat, Dec 1, 2018 at 12:38 PM Cong Wang <xiyou.wangc...@gmail.com> wrote: > > > > is_last_ethertype_ip() is used to check IP/IPv6 protocol before > > parsing IP/IPv6 headers. > > > > But __vlan_get_protocol() is only bound to skb->len, a malicious > > packet could exhaust all skb->len by inserting sufficient ETH_P_8021AD > > headers, and it may not even contain an IP/IPv6 header at all, so we > > have to check if we are still safe to continue to parse IP/IPv6 header. > > If not, treat it as non-IP packet. > > > > This should not cause any crash as we stil have tail room in skb, > > but we can't just rely on it either. > > Hi Cong, is this reproducible or just a theory ? which part of the > code you think will cause the invalid access or crash ?
Since you don't even read into my changelog, here it is: "This should not cause any crash as we stil have tail room in skb, but we can't just rely on it either." As I already explained to you in a private email, when we reference whatever field in struct iphdr, we have to make sure the offset of that field is within skb->len. > do you have steps to reproduce this? > Again, you really have to read the changelog I wrote: "a malicious packet could exhaust all skb->len by inserting sufficient ETH_P_8021AD headers, and it may not even contain an IP/IPv6 header at all, " > I would like to investigate this myself, it will take a couple of days > if that's ok with you .. Sure, take your time. I am sending the patch only for showing the problem, NOT to merge. Let's discard it anyway. I am wasting my time. Thanks.
