neighbour: Avoid writing before skb->head in neigh_hh_output() (e6ac64d4) · Commits · 戴 / test

While skb_push() makes the kernel panic if the skb headroom is less than the unaligned hardware header size, it will proceed normally in case we copy more than that because of alignment, and we'll silently corrupt adjacent slabs. In the case fixed by the previous patch, "ipv6: Check available headroom in ip6_xmit() even without options", we end up in neigh_hh_output() with 14 bytes headroom, 14 bytes hardware header and write 16 bytes, starting 2 bytes before the allocated buffer. Always check we're not writing before skb->head and, if the headroom is not enough, warn and drop the packet. v2: - instead of panicking with BUG_ON(), WARN_ON_ONCE() and drop the packet (Eric Dumazet) - if we avoid the panic, though, we need to explicitly check the headroom before the memcpy(), otherwise we'll have corrupted slabs on a running kernel, after we warn - use __skb_push() instead of skb_push(), as the headroom check is already implemented here explicitly (Eric Dumazet) Signed-off-by:

Stefano Brivio <sbrivio@redhat.com> Signed-off-by:

David S. Miller <davem@davemloft.net>

include/net/neighbour.h

+23 −5

Original line number	Diff line number	Diff line
		@@ -454,6 +454,7 @@ static inline int neigh_hh_bridge(struct hh_cache hh, struct sk_buff skb)

		static inline int neigh_hh_output(const struct hh_cache hh, struct sk_buff skb)
		{
		unsigned int hh_alen = 0;
		unsigned int seq;
		unsigned int hh_len;

		@@ -461,16 +462,33 @@ static inline int neigh_hh_output(const struct hh_cache hh, struct sk_buff skb
		seq = read_seqbegin(&hh->hh_lock);
		hh_len = hh->hh_len;
		if (likely(hh_len <= HH_DATA_MOD)) {
		hh_alen = HH_DATA_MOD;

		/* skb_push() would proceed silently if we have room for
		* the unaligned size but not for the aligned size:
		* check headroom explicitly.
		*/
		if (likely(skb_headroom(skb) >= HH_DATA_MOD)) {
		/* this is inlined by gcc */
		memcpy(skb->data - HH_DATA_MOD, hh->hh_data, HH_DATA_MOD);
		memcpy(skb->data - HH_DATA_MOD, hh->hh_data,
		HH_DATA_MOD);
		}
		} else {
		unsigned int hh_alen = HH_DATA_ALIGN(hh_len);
		hh_alen = HH_DATA_ALIGN(hh_len);

		memcpy(skb->data - hh_alen, hh->hh_data, hh_alen);
		if (likely(skb_headroom(skb) >= hh_alen)) {
		memcpy(skb->data - hh_alen, hh->hh_data,
		hh_alen);
		}
		}
		} while (read_seqretry(&hh->hh_lock, seq));

		skb_push(skb, hh_len);
		if (WARN_ON_ONCE(skb_headroom(skb) < hh_alen)) {
		kfree_skb(skb);
		return NET_XMIT_DROP;
		}

		__skb_push(skb, hh_len);
		return dev_queue_xmit(skb);
		}

Admin message