Re: [PATCH] net: prevent corruption of skb when using skb_gso_segment

From: Florian Westphal
Date: Thu Jan 07 2016 - 14:31:38 EST


Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxx> wrote:
> skb_gso_segment uses skb->cb, which may be owned by the caller. This may
> cause IPCB(skb)->opt.optlen to be overwritten, which will make
> ip_fragment overwrite skb data and possibly skb_shinfo with IPOPT_NOOP,
> thus causing a crash.
>
> This patch saves skb->cb before calling skb_gso_segment for those users
> that have anything to save, then restore it for each GSO segment.

Thanks.

> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -34,6 +34,7 @@
> #include <net/tcp_states.h>
> #include <net/netfilter/nf_queue.h>
> #include <net/netns/generic.h>
> +#include <net/ip.h>
>
> #include <linux/atomic.h>
>
> @@ -678,6 +679,10 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
> int err = -ENOBUFS;
> struct net *net = entry->state.net;
> struct nfnl_queue_net *q = nfnl_queue_pernet(net);
> + union {
> + struct inet_skb_parm h4;
> + struct inet6_skb_parm h6;
> + } header;
>
> /* rcu_read_lock()ed by nf_hook_slow() */
> queue = instance_lookup(q, queuenum);
> @@ -702,6 +707,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
> return __nfqnl_enqueue_packet(net, queue, entry);
>
> nf_bridge_adjust_skb_data(skb);
> + memcpy(&header, skb->cb, sizeof(header));
> segs = skb_gso_segment(skb, 0);
> /* Does not use PTR_ERR to limit the number of error codes that can be
> * returned by nf_queue. For instance, callers rely on -ESRCH to
> @@ -713,6 +719,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
> err = 0;
> do {
> struct sk_buff *nskb = segs->next;
> + memcpy(skb->cb, &header, sizeof(header));

I think this should be 'segs->cb'.

Other than that, this looks good to me.

> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -166,7 +166,12 @@ static int xfrm_output2(struct net *net, struct sock *sk, struct sk_buff *skb)
> @@ -179,6 +184,7 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb
> int err;
>
> segs->next = NULL;
> + memcpy(skb->cb, &header, sizeof(header));
> err = xfrm_output2(net, sk, segs);

likewise.