From patchwork Thu Aug 9 21:29:03 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 176278 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 A65C82C008D for ; Fri, 10 Aug 2012 07:29:31 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757357Ab2HIV3O (ORCPT ); Thu, 9 Aug 2012 17:29:14 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:52531 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756562Ab2HIV3J (ORCPT ); Thu, 9 Aug 2012 17:29:09 -0400 Received: by wibhq12 with SMTP id hq12so678702wib.1 for ; Thu, 09 Aug 2012 14:29:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; bh=QhfJ80i0y2jFvh1GsofC11gsl7qiWoT7gkGaTJhGvv0=; b=nTevKTlgfpbbNW1G5bmDVYD3WG02Zx8qayAZhgRC9B3ZSe3Vwbp6/F1gi/7LSjYrb9 mapxC7xUVSeTQggk00w9p8okyDpTD4832G+BrYkONxsS9gf6IMMxsUiIJJF61NEi5rdu l36E5faJ+WbGT04vonrLk736Xyik2PmvA0DdPqaXawFL9dp9ft/crTPVTaLz85KMzGlo dZ8c8ZoSM6kcbS+ULVaoP4cMwaFccwacsA31l3Nbp0t50Ch1ManrvFBU9ykWNmDst9ke SATj7fCPen7Ygtcl+tq0NJb+AXhE7n0M/bPbB2Q4TWfRrgvOniKmYqtPXTQDjxT+7SA7 4f9w== Received: by 10.216.60.208 with SMTP id u58mr319058wec.84.1344547747803; Thu, 09 Aug 2012 14:29:07 -0700 (PDT) Received: from [172.28.90.230] ([74.125.122.49]) by mx.google.com with ESMTPS id h9sm3949497wiz.1.2012.08.09.14.29.05 (version=SSLv3 cipher=OTHER); Thu, 09 Aug 2012 14:29:06 -0700 (PDT) Subject: Re: [PATCH] ipv4: tcp: security_sk_alloc() needed for unicast_sock From: Eric Dumazet To: Eric Paris Cc: Paul Moore , David Miller , Casey Schaufler , John Stultz , "Serge E. Hallyn" , lkml , James Morris , selinux@tycho.nsa.gov, john.johansen@canonical.com, LSM , netdev In-Reply-To: References: <50215A7E.8000701@linaro.org> <1344462889.28967.328.camel@edumazet-glaptop> <5022FD9A.4020603@schaufler-ca.com> <1695034.0lrQgQPOMT@sifl> <1344523833.28967.996.camel@edumazet-glaptop> Date: Thu, 09 Aug 2012 23:29:03 +0200 Message-ID: <1344547743.31104.582.camel@edumazet-glaptop> 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 On Thu, 2012-08-09 at 16:06 -0400, Eric Paris wrote: > NAK. > > I personally think commit be9f4a44e7d41cee should be reverted until it > is fixed. Let me explain what all I believe it broke and how. > Suggesting to revert this commit while we have known working fixes is a bit of strange reaction. I understand you are upset, but I believe we tried to fix it. > Old callchain of the creation of the 'equivalent' socket previous to > the patch in question just for reference: > > inet_ctl_sock_create > sock_create_kern > __sock_create > pf->create (inet_create) > sk_alloc > sk_prot_alloc > security_sk_alloc() > > > This WAS working properly. All of it. Nobody denies it. But acknowledge my patch uncovered a fundamental issue. What kind of 'security module' can decide to let RST packets being sent or not, on a global scale ? (one socket for the whole machine) smack_sk_alloc_security() uses smk_of_current() : What can be the meaning of smk_of_current() in the context of 'kernel' sockets... Your patch tries to maintain this status quo. In fact I suggest the following one liner patch, unless you can really demonstrate what can be the meaning of providing a fake socket for these packets. This mess only happened because ip_append_data()/ip_push_pending_frames() are so complex and use an underlying socket. But this socket should not be ever used outside of its scope. --- 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/ipv4/ip_output.c b/net/ipv4/ip_output.c index 76dde25..ec410e0 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -1536,6 +1536,7 @@ void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr, arg->csumoffset) = csum_fold(csum_add(nskb->csum, arg->csum)); nskb->ip_summed = CHECKSUM_NONE; + skb_orphan(nskb); skb_set_queue_mapping(nskb, skb_get_queue_mapping(skb)); ip_push_pending_frames(sk, &fl4); }