netdev - hard_header_len vs needed

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHA+R7Nyi9TvpN+fJrFt7krhLLXyJ5ify1ONvAse+rJFU0kbKg@mail.gmail.com>
Date:	Thu, 5 Jun 2014 16:39:09 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	Pravin B Shelar <pshelar@...ira.com>
Cc:	David Miller <davem@...emloft.net>,
	Timo Teräs <timo.teras@....fi>,
	netdev <netdev@...r.kernel.org>
Subject: hard_header_len vs needed_headroom

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.

What do you think?

Thanks.

View attachment "tunnel-hard_header_len.diff" of type "text/plain" (3346 bytes)