Skip to content

Commit

Permalink
net: Clone skb before setting peeked flag
Browse files Browse the repository at this point in the history
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: a59322b ("[UDP]: Only increment counter on first peek/recv")
Reported-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Herbert Xu authored and David S. Miller committed Jul 15, 2015
1 parent 035d210 commit 738ac1e
Showing 1 changed file with 38 additions and 3 deletions.
41 changes: 38 additions & 3 deletions net/core/datagram.c
Original file line number Diff line number Diff line change
@@ -131,6 +131,35 @@ static int wait_for_more_packets(struct sock *sk, int *err, long *timeo_p,
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
* @sk: socket
@@ -165,7 +194,9 @@ static int wait_for_more_packets(struct sock *sk, int *err, long *timeo_p,
struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
int *peeked, int *off, int *err)
{
struct sk_buff_head *queue = &sk->sk_receive_queue;
struct sk_buff *skb, *last;
unsigned long cpu_flags;
long timeo;
/*
* Caller is allowed not to check sk->sk_err before skb_recv_datagram()
@@ -184,8 +215,6 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
* Look at current nfs client by the way...
* However, this function was correct in any case. 8)
*/
unsigned long cpu_flags;
struct sk_buff_head *queue = &sk->sk_receive_queue;
int _off = *off;

last = (struct sk_buff *)queue;
@@ -199,7 +228,11 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
_off -= skb->len;
continue;
}
skb->peeked = 1;

error = skb_set_peeked(skb);
if (error)
goto unlock_err;

atomic_inc(&skb->users);
} else
__skb_unlink(skb, queue);
@@ -223,6 +256,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,

return NULL;

unlock_err:
spin_unlock_irqrestore(&queue->lock, cpu_flags);
no_packet:
*err = error;
return NULL;

0 comments on commit 738ac1e

Please sign in to comment.