Re: [PATCH] xfrm: validate ihl in xfrm4_transport_output()

[Qihang <q.h.hack.winter@gmail.com>]

Hi Sashiko,

Thanks for the review.  Both points are valid; I will send a v2.

[High] pskb_may_pull() instead of skb->len

You are right that skb->len is too loose.  __skb_pull() only needs the
header to be in the linear area: it BUG()s when the pull drives skb->len
below skb->data_len, i.e. when ihl > skb_headlen(skb).  skb->len counts
the paged fragments too, so an AF_PACKET packet with a virtio net header
and hdr_len = 1 leaves skb_headlen() == 1 while skb->len can be large;
ihl values up to skb->len then pass the ihl > skb->len check and still
reach the BUG() (and the memmove() source runs past skb->tail).  v2 uses
pskb_may_pull(skb, ihl): it linearizes the header, which both satisfies
__skb_pull()'s precondition and keeps the memmove() source in bounds,
and it rejects ihl > skb->len.  ip_hdr() is re-read afterwards because
pskb_may_pull() may reallocate the skb head.

[Medium] lower bound on ihl

Agreed, thanks.  v2 also rejects ihl < 5, matching ip_rcv_core() and the
implicit assumption in xfrm4_extract_header() and the IPv4 BEET paths.
This keeps the mac_header protocol byte initialized before esp_output()
reads it, so the uninitialized-headroom path you described is closed as
well.

The check is now:

	if (iph->ihl < 5 || !pskb_may_pull(skb, ihl))
		return -EINVAL;

	iph = ip_hdr(skb);

v2 sent separately as [PATCH v2].

pw-bot: cr