From patchwork Wed Jul 21 08:43:55 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 59411 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 8C67DB70A5 for ; Wed, 21 Jul 2010 18:44:27 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933753Ab0GUIoG (ORCPT ); Wed, 21 Jul 2010 04:44:06 -0400 Received: from mail-ww0-f44.google.com ([74.125.82.44]:43248 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933744Ab0GUIn6 (ORCPT ); Wed, 21 Jul 2010 04:43:58 -0400 Received: by wwj40 with SMTP id 40so1996524wwj.1 for ; Wed, 21 Jul 2010 01:43:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=F7/2QWHkSy9cyhOb5Dh6zkF6Ve3lCemM2eDlwOUR5CI=; b=nx855nW1ZJUHFN+BfCyzgiXfHd+lnrhExU6ucJxCZwfp05wrRJC2Ec5oHvxhSQgrpC h8LsEZpUDGQg7kU+FUI4KKQU7+qh4fNK1CTweLbI/1dpier7MK+mv0aILZpG7jtRL6iG ljEWqk3ZjV/qaRfete4sDTLhm08ZVigwgPrM8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=FMKuu/d8YOPXXDzSWoep1RQNdN5YjUthBelToHqtO+NRLg31Tg1qLKJqH8fXbC/jPT rC2s/17/jpZk4ZFrOPlq39s0g5NpSAGbVoSnHw9/v5hynFXomFrmp7uSdYGiqp+kuxI7 QX6DSDPogjY6h0PxgMRFXOefhexULlstJV8dU= Received: by 10.227.156.14 with SMTP id u14mr4055393wbw.55.1279701837308; Wed, 21 Jul 2010 01:43:57 -0700 (PDT) Received: from [127.0.0.1] ([85.17.35.125]) by mx.google.com with ESMTPS id e31sm51203348wbe.17.2010.07.21.01.43.56 (version=SSLv3 cipher=RC4-MD5); Wed, 21 Jul 2010 01:43:56 -0700 (PDT) Subject: Re: [PATCH net-next-2.6] netlink: netlink_recvmsg() fix From: Eric Dumazet To: Johannes Berg Cc: David Miller , netdev In-Reply-To: <1279700420.2452.15.camel@edumazet-laptop> References: <1279631789.2498.71.camel@edumazet-laptop> <1279639232.2498.82.camel@edumazet-laptop> <1279699529.3707.5.camel@jlt3.sipsolutions.net> <1279700420.2452.15.camel@edumazet-laptop> Date: Wed, 21 Jul 2010 10:43:55 +0200 Message-ID: <1279701835.2452.17.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le mercredi 21 juillet 2010 à 10:20 +0200, Eric Dumazet a écrit : > Nothing can touch this skb at the same time but us and our friends > (consumers that did a skb_recv_datagram( MSG_PEEK ) operation). > > Oh well, I see skb_unshare() tests skb_cloned(). This is not what we > want. > Here is V3 of this patch. Thanks [PATCH net-next-2.6 v3] netlink: netlink_recvmsg() fix commit 1dacc76d0014 (net/compat/wext: send different messages to compat tasks) introduced a race condition on netlink, in case MSG_PEEK is used. An skb given by skb_recv_datagram() might be shared, we must copy it before any modification, or risk fatal corruption. Signed-off-by: Eric Dumazet --- net/netlink/af_netlink.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 7aeaa83..0c86657 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1405,7 +1405,7 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock, struct netlink_sock *nlk = nlk_sk(sk); int noblock = flags&MSG_DONTWAIT; size_t copied; - struct sk_buff *skb, *frag __maybe_unused = NULL; + struct sk_buff *skb; int err; if (flags&MSG_OOB) @@ -1440,7 +1440,21 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock, kfree_skb(skb); skb = compskb; } else { - frag = skb_shinfo(skb)->frag_list; + /* + * Before setting frag_list to NULL, we must get a + * private copy of skb if shared (because of MSG_PEEK) + */ + if (skb_shared(skb) { + struct sk_buff *nskb; + + nskb = pskb_copy(skb, GFP_KERNEL); + kfree_skb(skb); + skb = nskb; + err = -ENOMEM; + if (!skb) + goto out; + } + kfree_skb(skb_shinfo(skb)->frag_list); skb_shinfo(skb)->frag_list = NULL; } } @@ -1477,10 +1491,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock, if (flags & MSG_TRUNC) copied = skb->len; -#ifdef CONFIG_COMPAT_NETLINK_MESSAGES - skb_shinfo(skb)->frag_list = frag; -#endif - skb_free_datagram(sk, skb); if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2)