lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 9 Jun 2014 16:37:01 -0700 From: Cong Wang <cwang@...pensource.com> To: Pravin Shelar <pshelar@...ira.com> Cc: David Miller <davem@...emloft.net>, Timo Teräs <timo.teras@....fi>, netdev <netdev@...r.kernel.org> Subject: Re: hard_header_len vs needed_headroom On Mon, Jun 9, 2014 at 11:16 AM, Pravin Shelar <pshelar@...ira.com> wrote: > On Thu, Jun 5, 2014 at 4:39 PM, Cong Wang <cwang@...pensource.com> wrote: >> Hello, >> >> >> While debugging a skb header push crash, I noticed that >> new ip tunnel code uses dev->needed_headroom as the >> length of hardware header: >> >> dev->needed_headroom = t_hlen + hlen; >> >> in IPIP tunnel case it's the ethernet header length plus IPv4 >> header length. And it leaves dev->hard_header_len as 0. >> >> But there are some places assuming >> dev->hard_header_len is the length of L2 header, at least >> tcf_mirred() calls skb_push(skb, skb->dev->hard_header_len) >> to reset skb->data back to L2 header. Current code is broken. >> >> I think ip tunnel code is wrong, according to LL_RESERVED_SPACE(), >> it seems dev->hard_header_len should be the L2 header while >> dev->needed_headroom should be the *additional* headroom needed >> by hardware. That also means skb_push(dev->hard_header_len) is >> correct. >> >> Therefore, I think IP tunnel should set its dev->hard_header_len to >> lower_dev->hard_header_len + tunnel->hlen + sizeof(struct iphdr), >> and set its dev->needed_headroom to lower_dev->needed_headroom. >> See the attached patch for details. >> >> But gretap seems to be an exception, it calls ether_setup() to init its >> dev->hard_header_len to ETH_HLEN. My patch probably breaks it, >> I think it should set its tunnel->hlen = gre_hdr_len + ETH_HLEN. >> > > When ip-tunnel was written I thought LL_RESERVED_SPACE() is used to > reserve header space. Obviously I missed tcf_mirred() It should be used to reserve headroom, however, mirred needs ->hard_header_len to reset skb->data back to mac header from network header. > At this point I think easiest fix would be using LL_RESERVED_SPACE() > in tcf_mirred(). > Later we can change ip-tunnel code to use hard_header and > needed_headroom as you have menioned. > It's not whether LL_RESERVED_SPACE() is used or not, it's whether dev->hard_header_len is always the offset from mac_header to network_header. mirred is not alone, tpacket_fill_skb() uses this as well. So I think we have to fix dev->hard_header_len for tunnels. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists