Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751693AbbHJLhQ (ORCPT ); Mon, 10 Aug 2015 07:37:16 -0400 Received: from forward-corp1m.cmail.yandex.net ([5.255.216.100]:43411 "EHLO forward-corp1m.cmail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751621AbbHJLhN (ORCPT ); Mon, 10 Aug 2015 07:37:13 -0400 Authentication-Results: smtpcorp4.mail.yandex.net; dkim=pass header.i=@yandex-team.ru Message-ID: <55C88CE1.2030605@yandex-team.ru> Date: Mon, 10 Aug 2015 14:37:05 +0300 From: Konstantin Khlebnikov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Ben Hutchings , linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "David S. Miller" , Herbert Xu Subject: Re: [PATCH 3.2 089/110] net: Clone skb before setting peeked flag References: In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3469 Lines: 122 On 10.08.2015 13:12, Ben Hutchings wrote: > 3.2.71-rc1 review patch. If anyone has any objections, please let me know. Here is important fix: https://patchwork.ozlabs.org/patch/503374/ "net: Fix skb_set_peeked use-after-free". not in upstream yet. > > ------------------ > > From: Herbert Xu > > commit 738ac1ebb96d02e0d23bc320302a6ea94c612dec 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: a59322be07c9 ("[UDP]: Only increment counter on first peek/recv") > Reported-by: Konstantin Khlebnikov > Signed-off-by: Herbert Xu > Signed-off-by: David S. Miller > [bwh: Backported to 3.2: adjust context] > Signed-off-by: Ben Hutchings > --- > net/core/datagram.c | 41 ++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 38 insertions(+), 3 deletions(-) > > --- a/net/core/datagram.c > +++ b/net/core/datagram.c > @@ -128,6 +128,35 @@ out_noerr: > 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 > @@ -160,7 +189,9 @@ out_noerr: > struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags, > int *peeked, int *err) > { > + struct sk_buff_head *queue = &sk->sk_receive_queue; > struct sk_buff *skb; > + unsigned long cpu_flags; > long timeo; > /* > * Caller is allowed not to check sk->sk_err before skb_recv_datagram() > @@ -179,15 +210,16 @@ struct sk_buff *__skb_recv_datagram(stru > * 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; > - > spin_lock_irqsave(&queue->lock, cpu_flags); > skb = skb_peek(queue); > if (skb) { > *peeked = skb->peeked; > if (flags & MSG_PEEK) { > - skb->peeked = 1; > + > + error = skb_set_peeked(skb); > + if (error) > + goto unlock_err; > + > atomic_inc(&skb->users); > } else > __skb_unlink(skb, queue); > @@ -206,6 +238,8 @@ struct sk_buff *__skb_recv_datagram(stru > > return NULL; > > +unlock_err: > + spin_unlock_irqrestore(&queue->lock, cpu_flags); > no_packet: > *err = error; > return NULL; > -- Konstantin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/