Commit 52501955 authored by Herbert Xu's avatar Herbert Xu Committed by Willy Tarreau

net: Clone skb before setting peeked flag

commit 738ac1eb upstream.

Shared skbs must not be modified and this is crucial for broadcast
and/or multicast paths where we use it as an optimisation to avoid
unnecessary cloning.

The function skb_recv_datagram breaks this rule by setting peeked
without cloning the skb first.  This causes funky races which leads
to double-free.

This patch fixes this by cloning the skb and replacing the skb
in the list when setting skb->peeked.

Fixes: a59322be ("[UDP]: Only increment counter on first peek/recv")
Reported-by: default avatarKonstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
(cherry picked from commit 72e6f068)
[wt: adjusted context for 2.6.32. Introduces a bug, see next commit]
Signed-off-by: default avatarWilly Tarreau <w@1wt.eu>
parent 4ad8ce65
...@@ -127,6 +127,35 @@ static int wait_for_packet(struct sock *sk, int *err, long *timeo_p) ...@@ -127,6 +127,35 @@ static int wait_for_packet(struct sock *sk, int *err, long *timeo_p)
goto out; goto out;
} }
static int skb_set_peeked(struct sk_buff *skb)
{
struct sk_buff *nskb;
if (skb->peeked)
return 0;
/* We have to unshare an skb before modifying it. */
if (!skb_shared(skb))
goto done;
nskb = skb_clone(skb, GFP_ATOMIC);
if (!nskb)
return -ENOMEM;
skb->prev->next = nskb;
skb->next->prev = nskb;
nskb->prev = skb->prev;
nskb->next = skb->next;
consume_skb(skb);
skb = nskb;
done:
skb->peeked = 1;
return 0;
}
/** /**
* __skb_recv_datagram - Receive a datagram skbuff * __skb_recv_datagram - Receive a datagram skbuff
* @sk: socket * @sk: socket
...@@ -160,6 +189,7 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags, ...@@ -160,6 +189,7 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
int *peeked, int *err) int *peeked, int *err)
{ {
struct sk_buff *skb; struct sk_buff *skb;
unsigned long cpu_flags;
long timeo; long timeo;
/* /*
* Caller is allowed not to check sk->sk_err before skb_recv_datagram() * Caller is allowed not to check sk->sk_err before skb_recv_datagram()
...@@ -178,14 +208,16 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags, ...@@ -178,14 +208,16 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
* Look at current nfs client by the way... * Look at current nfs client by the way...
* However, this function was corrent in any case. 8) * However, this function was corrent in any case. 8)
*/ */
unsigned long cpu_flags;
spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags); spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
skb = skb_peek(&sk->sk_receive_queue); skb = skb_peek(&sk->sk_receive_queue);
if (skb) { if (skb) {
*peeked = skb->peeked; *peeked = skb->peeked;
if (flags & MSG_PEEK) { if (flags & MSG_PEEK) {
skb->peeked = 1;
error = skb_set_peeked(skb);
if (error)
goto unlock_err;
atomic_inc(&skb->users); atomic_inc(&skb->users);
} else } else
__skb_unlink(skb, &sk->sk_receive_queue); __skb_unlink(skb, &sk->sk_receive_queue);
...@@ -204,6 +236,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags, ...@@ -204,6 +236,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
return NULL; return NULL;
unlock_err:
spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
no_packet: no_packet:
*err = error; *err = error;
return NULL; return NULL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment