Fwd: Please test ipfw and pf uid/gid/jail rules

Tue Sep 30 06:15:28 UTC 2008

On Tue, 30 Sep 2008, Max Laier wrote:

> On Tuesday 30 September 2008 00:02:04 Robert Watson wrote:
>> On Mon, 29 Sep 2008, Max Laier wrote:
>>> Please help testing.  It's been confirmed to work for IPFW, let's make 
>>> sure pf is in good shape, too.  Thanks.
>>
>> A casual glance at pf.c suggests that pf(4) doesn't suffer from the "look 
>> up the inpcb even though it's passed down if the socket pointer is NULL" 
>> bug that ipfw(4) did, but confirmation that things work properly would 
>> definitely be good.
>
> http://www.freebsd.org/cgi/query-pr.cgi?pr=127439 looks like it could be 
> related.  I think I see what's happening there, but unfortunately I don't 
> have any time to look into it myself at the moment.  Might be a while before 
> I get to it so additional eyes are certainly appreciated!

There are a number of LOR's in this PR; some are harmless.  Here's a quick and 
casual run-down:

   1st 0xc0907fcc pf task mtx (pf task mtx) @
   /usr/src/sys/contrib/pf/net/pf_ioctl.c:1394
   2nd 0xc0973488 ifnet (ifnet) @ /usr/src/sys/net/if.c:1558

I don't know anything about this.

   1st 0xc097830c tcp (tcp) @ /usr/src/sys/netinet/tcp_input.c:400
   2nd 0xc09775d8 PFil hook read/write mutex (PFil hook read/write mutex) @
   /usr/src/sys/net/pfil.c:73

This should be fixed by one of my recent TCP changes -- TCP wasn't passing 
down the inpcb in a situation where it should have been.

   1st 0xc4013d44 udpinp (udpinp) @ /usr/src/sys/netinet/udp_usrreq.c:878
   2nd 0xc09775d8 PFil hook read/write mutex (PFil hook read/write mutex) @
   /usr/src/sys/net/pfil.c:73

This is the correct order.

   1st 0xc423f150 tcpinp (tcpinp) @ /usr/src/sys/netinet/tcp_usrreq.c:472
   2nd 0xc09775d8 PFil hook read/write mutex (PFil hook read/write mutex) @
   /usr/src/sys/net/pfil.c:73

This is the correct order.

   1st 0xc09786cc udp (udp) @ /usr/src/sys/netinet/udp_usrreq.c:395
   2nd 0xc09775d8 PFil hook read/write mutex (PFil hook read/write mutex) @
   /usr/src/sys/net/pfil.c:73

This is the correct order.

   panic: _rw_rlock (tcp): wlock already held @
   /usr/src/sys/contrib/pf/net/pf.c:3016
   cpuid = 0
   KDB: stack backtrace:
   db_trace_self_wrapper(c088cf61,e6846220,c05ae7df,c08b659d,0,...) at
   db_trace_self_wrapper+0x26
   kdb_backtrace(c08b659d,0,c0889c7e,e684622c,0,...) at kdb_backtrace+0x29
   panic(c0889c7e,c085a754,c088f55e,c087092d,bc8,...) at panic+0x10f
   _rw_rlock(c097830c,c087092d,bc8,c08d9624,c087092d,...) at _rw_rlock+0x73
   pf_socket_lookup(2,e68463dc,0,cc4,3,...) at pf_socket_lookup+0x208
   pf_test_tcp(e6846444,e6846440,2,c3efee00,c3c8e900,...) at pf_test_tcp+0x142
   pf_test6(2,c3d44000,e68464a0,0,0,...) at pf_test6+0x8a0
   pf_check6_out(0,e68464a0,c3d44000,2,0,...) at pf_check6_out+0x47
   pfil_run_hooks(c097ad00,e6846638,c3d44000,2,0,...) at pfil_run_hooks+0x88
   ip6_output(c3c8e900,0,e6846618,0,0,...) at ip6_output+0x122e
   pf_send_tcp(c4fcfe00,c41259b4,1c,c4fcfe5c,c4fcfe4c,...) at pf_send_tcp+0x6dd
   pf_test_tcp(e68468e8,e68468e4,2,c3f20900,c4fcfe00,...) at pf_test_tcp+0xcef
   pf_test6(2,c3f06400,e6846944,0,c446b7bc,...) at pf_test6+0x8a0
   pf_check6_out(0,e6846944,c3f06400,2,c446b7bc,...) at pf_check6_out+0x47
   pfil_run_hooks(c097ad00,e6846adc,c3f06400,2,c446b7bc,...) at
   pfil_run_hooks+0x88
   ip6_output(c4fcfe00,0,e6846abc,0,0,...) at ip6_output+0x122e
   tcp_output(c45553a0,c447e7c0,201,c446b858,c45553a0,...) at tcp_output+0x137e
   tcp6_usr_connect(c50cd340,c447e7c0,c4eed690,25,e6846c64,...) at
   tcp6_usr_connect+0x171
   soconnect(c50cd340,c447e7c0,c4eed690,1c,16,...) at soconnect+0x52
   kern_connect(c4eed690,3,c447e7c0,c447e7c0,0,...) at kern_connect+0x59
   connect(c4eed690,e6846cfc,c,c08a288e,c08d3a50,...) at connect+0x46
   syscall(e6846d38) at syscall+0x274

This looks like a recursion bug in pf(4) -- you can't look up sockets in that 
output context because youre already running in a context where connection 
locks are held.  If it's for the same TCP connection, you need to pass down 
the inpcb into ip6_output(), but if it's for a different one, you ned to run 
the output code in a deferred context so that it can recurse safely back into 
the inpcb code.

Robert N M Watson
Computer Laboratory
University of Cambridge

>
>> Thanks,
>>
>> Robert N M Watson
>> Computer Laboratory
>> University of Cambridge
>>
>>> ----------  Forwarded Message  ----------
>>>
>>> Subject: Please test ipfw and pf uid/gid/jail rules
>>> Date: Monday 29 September 2008
>>> From: Robert Watson <rwatson at freebsd.org>
>>> To: current at freebsd.org
>>>
>>>
>>> Dear all:
>>>
>>> Although it didn't show up in 8.x testing to date, it turned out there
>>> was a serious stability regression in the ipfw uid/gid/jail rule
>>> implementation as a result of moving to rwlocks for inpcbinfo and inpcb.
>>> I think I've corrected the sources of the problem in 8.x and 7.x now, but
>>> it would be very helpful if people who use ipfw and pf could do some
>>> extra testing of these rules with invariants and witness enabled to see
>>> if we can't shake out any remaining problems.
>>>
>>> Thanks,
>>>
>>> Robert N M Watson
>>> Computer Laboratory
>>> University of Cambridge
>>> -------------------------------------------------------
>>> --
>>> /"\  Best regards,                      | mlaier at freebsd.org
>>> \ /  Max Laier                          | ICQ #67774661
>>> X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
>>> / \  ASCII Ribbon Campaign              | Against HTML Mail and News
>
> -- 
> /"\  Best regards,                      | mlaier at freebsd.org
> \ /  Max Laier                          | ICQ #67774661
> X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
> / \  ASCII Ribbon Campaign              | Against HTML Mail and News
>