[PATCH] /proc/net/tcp + ipv6 hang

[PATCH] /proc/net/tcp + ipv6 hang

[Anders Gustafsson]

Hi,

this patch fixes an infinite loop when reading /proc/net/tcp and having
daemons listening on ipv6.

-- 
Anders Gustafsson - andersg@0x63.nu - http://0x63.nu/

You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================


ChangeSet@1.913, 2002-12-23 02:49:19+01:00, andersg@0x63.nu
  Fix infinite loop when reading /proc/net/tcp with ipv6-sockets.


 tcp_ipv4.c |    1 +
 1 files changed, 1 insertion(+)


diff -Nru a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
--- a/net/ipv4/tcp_ipv4.c	Mon Dec 23 02:55:45 2002
+++ b/net/ipv4/tcp_ipv4.c	Mon Dec 23 02:55:45 2002
@@ -2236,6 +2236,7 @@
 			goto get_req;
 		}
 		read_unlock_bh(&tp->syn_wait_lock);
+		sk = sk->next;
 	}
 	if (++st->bucket < TCP_LHTABLE_SIZE) {
 		sk = tcp_listening_hash[st->bucket];

===================================================================


This BitKeeper patch contains the following changesets:
1.913
## Wrapped with gzip_uu ##


begin 664 bkpatch1524
M'XL(`"%M!CX``]54WVO;,!!^MOZ*@SZ6V#I+=FP/EVS=3S98R.CS4.1+;)Q(
MP5:3#/S'3\E"6]*6LK*7G?1PTIU.G^[[T`7<]-05@3(5=?V27<!GV[LBX/M4
MA.;6KV?6^G54VS5%IZRHI<[0*IJWT7QE=\QG3973-6Q]M`@P%'<[[M>&BF#V
MX=/-M[<SQLH2KFMEEO2#')0E<[;;JE753Y2K5]:$KE.F7Y-3H;;KX2YUB#F/
M_4AP+'B2#IAR.1XT5HA*(E4\EEDJV0G>Y`3^[#S&WD0L,!D2@7G"W@.&.0K@
M<81Q%!^<0N8%YI<<"\[AK!Q<(HPX>P?_%O0UT_"QV4-C%HUI','*V@WL:C+0
MD:H:LX1HTUD=&7*1TS[4N!J:S38=]5:WY/J0?87$%Y-L>M]=-OI+8XPKSJY>
M>-T!A+];'I#\/#BA?OC./,D&*64Z'GRGY\FBRN8YZ92+Q7DWGZWD:1(<I2=F
M2)&/\2B:)Y)?EL^KL3*EUS31UI!VS?98+)QWSR/&6!X0QWQ(\BS%H[!$]DA7
MXC_4U1\*OL.HVQVGU\GT*39>(;<OGN@,D`5!WT()?3NZ,K1W;^Z_$UV3;OO;
1=4D9*2FJ!?L-758XOZ\$````
`
end

Re: [PATCH] /proc/net/tcp + ipv6 hang

[Arnaldo Carvalho de Melo]

Em Mon, Dec 23, 2002 at 02:57:23AM +0100, Anders Gustafsson escreveu:
> Hi,
> 
> this patch fixes an infinite loop when reading /proc/net/tcp and having
> daemons listening on ipv6.

I'm checking your patch, just received the message, but thanks a lot for 
doing this!

- Arnaldo

Re: [PATCH] /proc/net/tcp + ipv6 hang

[Arnaldo Carvalho de Melo]

Em Mon, Dec 23, 2002 at 02:57:23AM +0100, Anders Gustafsson escreveu:
> this patch fixes an infinite loop when reading /proc/net/tcp and having
> daemons listening on ipv6.

Perfect! Thanks for the fix, looking at it now it seems soooo obvious, /me
slaps himself in the face 8)

David, pleasey apply, I think there is still at least one bug with this code,
will be testing this as soon as possible.

Anders, if you're feeling brave, from the top of my head, think about what
happens if somebody only reads the first, say, 10 bytes of /proc/net/tcp, will
we unlocking a not held lock at tcp_seq_stop, no? :-)

Another fix for this one will be apreciated 8)

- Arnaldo

Re: [PATCH] /proc/net/tcp + ipv6 hang

[Anders Gustafsson]

On Mon, Dec 23, 2002 at 12:40:17AM -0200, Arnaldo Carvalho de Melo wrote:
> 
> Anders, if you're feeling brave, from the top of my head, think about what
> happens if somebody only reads the first, say, 10 bytes of /proc/net/tcp, will
> we unlocking a not held lock at tcp_seq_stop, no? :-)

Yes, I was just looking into the locking... But it's rather messy with locks
between calls and goto's and I think I'd better get some sleep before saying
anything for certain. Is there any reason holding the lock between
listening_get_first() and the first call to listening_get_next(), but not
between consecutive calls to listening_get_next()? Otherwise we could just
always release the lock in listening_get_first.

(All this applies to established_get_first/next too.)

OOPS, I just realizes we might be talking about different locks :)

I was talking about 
read_[un]lock_bh(&tp->syn_wait_lock); in listening_get_first/next

What lock are you talking about?
As far as I can see, in TCP_SEQ_STATE_OPENREQ tp->syn_wait_lock is always
held and in TCP_SEQ_STATE_LISTENING the tcp_listen_lock and so on?

-- 
Anders Gustafsson - andersg@0x63.nu - http://0x63.nu/

Re: [PATCH] /proc/net/tcp + ipv6 hang

[Arnaldo Carvalho de Melo]

Em Mon, Dec 23, 2002 at 04:08:12AM +0100, Anders Gustafsson escreveu:
> On Mon, Dec 23, 2002 at 12:40:17AM -0200, Arnaldo Carvalho de Melo wrote:
 
> > Anders, if you're feeling brave, from the top of my head, think about what
> > happens if somebody only reads the first, say, 10 bytes of /proc/net/tcp,
> > will we unlocking a not held lock at tcp_seq_stop, no? :-)
 
> Yes, I was just looking into the locking... But it's rather messy with locks
> between calls and goto's and I think I'd better get some sleep before saying
> anything for certain. Is there any reason holding the lock between
> listening_get_first() and the first call to listening_get_next(), but not
> between consecutive calls to listening_get_next()? Otherwise we could just
> always release the lock in listening_get_first.
 
> (All this applies to established_get_first/next too.)

We have to hold the lock (tp->syn_wait_lock) because in listening_get_first
we return one of the elements of the list guarded by tp->syn_wait_lock, so
we have to make sure that it doesn't disappears under our feet when we go
to the next one in listening_get_next
 
> OOPS, I just realizes we might be talking about different locks :)

different problem :-)
 
> I was talking about 
> read_[un]lock_bh(&tp->syn_wait_lock); in listening_get_first/next
 
> What lock are you talking about?
> As far as I can see, in TCP_SEQ_STATE_OPENREQ tp->syn_wait_lock is always
> held and in TCP_SEQ_STATE_LISTENING the tcp_listen_lock and so on?

Notice that we call tcp_listen_lock() only in tcp_get_idx, and tcp_get_idx
is not called for the first record, that is the header of /proc/net/tcp,
we're in TCP_SEQ_STATE_LISTENING that is zero (we zeroed the private seq_file
area in tcp_seq_open, so if the userlevel program only asks for, say, four
bytes (like midnight commander's editor, to read the magic number for the file
to decide which viewer is associated with the type indicated by the magic
number) this will be satisfied with just one call to tcp_seq_show, i.e.
only when tcp_seq_start returns (void *), without calling tcp_get_idx to
call tcp_listen_lock(). b00m, we drop a lock not held, got it?

- Arnaldo

Re: [PATCH] /proc/net/tcp + ipv6 hang

[David S. Miller]

   From: Arnaldo Carvalho de Melo <acme@conectiva.com.br>
   Date: Mon, 23 Dec 2002 00:40:17 -0200
   
   David, pleasey apply, I think there is still at least one bug with this code,
   will be testing this as soon as possible.

Done, thanks.