lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 12 Sep 2007 16:37:16 +0200
From:	Wolfgang Walter <>
To:	"J. Bruce Fields" <>
Subject: Re: [NFS] [patch] sunrpc: make closing of old temporary sockets work (was: problems with lockd in

Am Mittwoch, 12. September 2007 15:37 schrieb J. Bruce Fields:
> On Wed, Sep 12, 2007 at 02:07:10PM +0200, Wolfgang Walter wrote:
> > as already described old temporary sockets (client is gone) of lockd
> > aren't closed after some time. So, with enough clients and some time
> > gone, there are 80 open dangling sockets and you start getting messages
> > of the form:
> >
> > lockd: too many open TCP sockets, consider increasing the number of nfsd
> > threads.
> Thanks for working on this problem!
> > If I understand the code then the intention was that the server closes
> > temporary sockets after about 6 to 12 minutes:
> >
> > 	a timer is started which calls svc_age_temp_sockets every 6 minutes.
> >
> > 	svc_age_temp_sockets:
> > 		if a socket is marked OLD it gets closed.
> > 		sockets which are not marked as OLD are marked OLD
> >
> > 	every time the sockets receives something OLD is cleared.
> >
> > But svc_age_temp_sockets never closes any socket though because it only
> > closes sockets with svsk->sk_inuse == 0. This seems to be a bug.
> >
> > Here is a patch against which changes the test to
> > svsk->sk_inuse <= 0 which was probably meant. The patched kernel runs
> > fine here. Unused sockets get closed (after 6 to 12 minutes)
> So the fact that this changes the behavior means that sk_inuse is taking
> on negative values.  This can't be right--how can something like
> svc_sock_put() (which does an atomic_dec_and_test) work in that case?

You probably misread the code.

if (atomic_read(&svsk->sk_inuse) || test_bit(SK_BUSY, &svsk->sk_flags))

This means: any socket where svsk->sk_inuse != 0 or SK_BUSY is set is ignored
by svc_age_temp_sockets: no attempt is made to close the svc.

This seems to be wrong: if svsk->sk_inuse is zero only if svc_delete_socket
has been called for it and will be deleted anyway (probably it is already
closed then).

But the intention of svc_age_temp_sockets is to close open temporary
sockets where no traffic has been received for more than 6 minutes. These
sockets have svsk->sk_inuse >= 1.

My patch does exactly this:

instead of

"skip sockets which are not already deleted or which are busy"


"skip sockets which are already deleted or which are busy"

> I wish I had time today to figure out what's going on in this case.  But
> from a quick through svsock.c for sk_inuse, it looks odd; I'm suspicious
> of anything without the stereotyped behavior--initializing to one,
> atomic_inc()ing whenever someone takes a reference, and
> atomic_dec_and_test()ing whenever someone drops it....

Then svc_tcp_accept would be wrong, too (it closes sockets the same way just
without testing for sk_inuse and SK_BUSY).

I think this works because as long as a socket is in sv_tempsocks or
sv_permsocks svsk->sk_inuse can never reach zero. As svc_age_temp_sockets locks
the list nobody can bring svsk->sk_inuse to zero as long as
svc_age_temp_sockets holds the lock. As svc_age_temp_sockets calls
atomic_inc(&svsk->sk_inuse) when holding the lock there is no
problem. (the same is true for svc_tcp_accept).

This is the reason why I doubt that this check for svsk->sk_inuse in
svc_age_temp_sockets is usefull at all. It should be always false.

Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
More majordomo info at

Powered by blists - more mailing lists