Re: [PATCH net-next 3/3] psp: validate IPv4 header fields in psp_dev_rcv()

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Jakub Kicinski wrote:
> psp_dev_rcv() is called from the NIC driver's RX completion path
> before the frame reaches ip_rcv_core(), so the IP header has not
> been validated in SW, yet. We expect that the device has done
> all this validation, but let's also add the SW checks, to avoid
> surprises.

If devices are expected to have verified this, should these be more
noisy checks, similar to netdev_rx_csum_fault?

 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/psp/psp_main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index f069117c867a..524978dfb8fd 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c
> @@ -300,6 +300,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
>  	if (proto == htons(ETH_P_IP)) {
>  		struct iphdr *iph = (struct iphdr *)(skb->data + l2_hlen);
>  
> +		if (unlikely(iph->ihl < 5))
> +			return -EINVAL;
> +
>  		is_udp = iph->protocol == IPPROTO_UDP;
>  		l3_hlen = iph->ihl * 4;
>  		if (l3_hlen != sizeof(struct iphdr) &&
> @@ -335,6 +338,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
>  	if (proto == htons(ETH_P_IP)) {
>  		struct iphdr *iph = (struct iphdr *)(skb->data + l2_hlen);
>  
> +		if (unlikely(ntohs(iph->tot_len) < l3_hlen + encap))
> +			return -EINVAL;
> +
>  		iph->protocol = psph->nexthdr;
>  		iph->tot_len = htons(ntohs(iph->tot_len) - encap);
>  		iph->check = 0;
> @@ -342,6 +348,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
>  	} else {
>  		struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
>  
> +		if (unlikely(ntohs(ipv6h->payload_len) < encap))
> +			return -EINVAL;
> +
>  		ipv6h->nexthdr = psph->nexthdr;
>  		ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) - encap);
>  	}
> -- 
> 2.54.0
>