From owner-freebsd-current@FreeBSD.ORG  Fri Jul 27 17:51:34 2007
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 623CE16A41B;
	Fri, 27 Jul 2007 17:51:34 +0000 (UTC)
	(envelope-from rwatson@FreeBSD.org)
Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42])
	by mx1.freebsd.org (Postfix) with ESMTP id 2EC2813C4E5;
	Fri, 27 Jul 2007 17:51:34 +0000 (UTC)
	(envelope-from rwatson@FreeBSD.org)
Received: from fledge.watson.org (fledge.watson.org [209.31.154.41])
	by cyrus.watson.org (Postfix) with ESMTP id 92F2E49AB6;
	Fri, 27 Jul 2007 13:51:33 -0400 (EDT)
Date: Fri, 27 Jul 2007 18:51:33 +0100 (BST)
From: Robert Watson <rwatson@FreeBSD.org>
X-X-Sender: robert@fledge.watson.org
To: "Christian S.J. Peron" <csjp@FreeBSD.org>
In-Reply-To: <20070727172053.GA49562@sub>
Message-ID: <20070727184958.K72112@fledge.watson.org>
References: <12A5576E06117043AB644E4A998703B7C1F877@Exc01.seccuris.local>
	<20070727172053.GA49562@sub>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Cc: current@freebsd.org
Subject: Re: Removing NET_NEEDS_GIANT: first patch
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>, 
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 27 Jul 2007 17:51:34 -0000


On Fri, 27 Jul 2007, Christian S.J. Peron wrote:

>> Index: ip_divert.c
>> ===================================================================
>> RCS file: /data/fbsd-cvs/ncvs/src/sys/netinet/ip_divert.c,v
>> retrieving revision 1.128
>> diff -u -r1.128 ip_divert.c
>> --- ip_divert.c	11 May 2007 10:20:50 -0000	1.128
>> +++ ip_divert.c	27 Jul 2007 14:25:09 -0000
>> @@ -61,6 +61,7 @@
>>   #include <vm/uma.h>
>>
>>   #include <net/if.h>
>> +#include <net/netisr.h>
>>   #include <net/route.h>
>>
>>   #include <netinet/in.h>
>> @@ -378,7 +379,7 @@
>>   				    ((so->so_options & SO_DONTROUTE) ?
>>   				    IP_ROUTETOIF : 0) |
>>   				    IP_ALLOWBROADCAST | IP_RAWOUTPUT,
>> -				    inp->inp_moptions, NULL);
>> +				    inp->inp_moptions, inp);
>
> Here we are passing the inp associated with the divert socket.  I am not 
> sure how accurate this is, since it's quite possible that the packet could 
> belong to another TCP/UDP socket owned by a completely different user and 
> socket for that matter.
>
> This will result in the firewalls attributing the packet to the user of 
> whoever created the divert socket (probably root), instead of attributing 
> the packet to the subject who created the original TCP/UDP socket.

Hmm, you are, of course, entirely right.  This would fix the locking problem 
but lead to incorrect results in processing.  The right fix here may be to 
arrange to drop all the locks, which means doing an m_dup of the options, 
before calling ip_output(), which would allow ip_output() to acquire whatever 
locks it needs.

I wonder if we have some similar issues with raw ICMP sockets?

Robert N M Watson
Computer Laboratory
University of Cambridge