[PATCH] sunrpc: fix refcounting problems in rpc servers

A recent patch fixed a problem which would occur when the refcount on an
auth_domain reached zero. This problem has not been reported in practice
despite existing in two major kernel releases because the refcount can
never reach zero.

This patch fixes the problems that stop the refcount reaching zero.

1/ We were adding to the refcount when inserting in the hash table,
but only removing from the hashtable when the refcount reached zero.
Obviously it never would. So don't count the implied reference of
being in the hash table.

2/ There are two paths on which a socket can be destroyed. One called
svcauth_unix_info_release(). The other didn't. So when the other was
taken, we can lose a reference to an ip_map which in-turn holds a
reference to an auth_domain

So unify the exit paths into svc_sock_put. This highlights the fact
that svc_delete_socket has slightly odd semantics - it does not drop
a reference but probably should. Fixing this need a bit more
thought and testing.

Signed-off-by: Neil Brown <neilb@suse.de>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

authored by Neil Brown and committed by Linus Torvalds d6740df9 2b52c959

+15 -19
+1 -3
net/sunrpc/svcauth.c
··· 147 return hp; 148 } 149 } 150 - if (new) { 151 hlist_add_head(&new->hash, head); 152 - kref_get(&new->ref); 153 - } 154 spin_unlock(&auth_domain_lock); 155 return new; 156 }
··· 147 return hp; 148 } 149 } 150 + if (new) 151 hlist_add_head(&new->hash, head); 152 spin_unlock(&auth_domain_lock); 153 return new; 154 }
+14 -16
net/sunrpc/svcsock.c
··· 300 svc_sock_put(struct svc_sock *svsk) 301 { 302 if (atomic_dec_and_test(&svsk->sk_inuse) && test_bit(SK_DEAD, &svsk->sk_flags)) { 303 - dprintk("svc: releasing dead socket\n"); 304 - sock_release(svsk->sk_sock); 305 kfree(svsk); 306 } 307 } ··· 1609 if (test_bit(SK_TEMP, &svsk->sk_flags)) 1610 serv->sv_tmpcnt--; 1611 1612 - if (!atomic_read(&svsk->sk_inuse)) { 1613 - spin_unlock_bh(&serv->sv_lock); 1614 - if (svsk->sk_sock->file) 1615 - sockfd_put(svsk->sk_sock); 1616 - else 1617 - sock_release(svsk->sk_sock); 1618 - if (svsk->sk_info_authunix != NULL) 1619 - svcauth_unix_info_release(svsk->sk_info_authunix); 1620 - kfree(svsk); 1621 - } else { 1622 - spin_unlock_bh(&serv->sv_lock); 1623 - dprintk(KERN_NOTICE "svc: server socket destroy delayed\n"); 1624 - /* svsk->sk_server = NULL; */ 1625 - } 1626 } 1627 1628 /*
··· 300 svc_sock_put(struct svc_sock *svsk) 301 { 302 if (atomic_dec_and_test(&svsk->sk_inuse) && test_bit(SK_DEAD, &svsk->sk_flags)) { 303 + printk("svc: releasing dead socket\n"); 304 + if (svsk->sk_sock->file) 305 + sockfd_put(svsk->sk_sock); 306 + else 307 + sock_release(svsk->sk_sock); 308 + if (svsk->sk_info_authunix != NULL) 309 + svcauth_unix_info_release(svsk->sk_info_authunix); 310 kfree(svsk); 311 } 312 } ··· 1604 if (test_bit(SK_TEMP, &svsk->sk_flags)) 1605 serv->sv_tmpcnt--; 1606 1607 + /* This atomic_inc should be needed - svc_delete_socket 1608 + * should have the semantic of dropping a reference. 1609 + * But it doesn't yet.... 1610 + */ 1611 + atomic_inc(&svsk->sk_inuse); 1612 + spin_unlock_bh(&serv->sv_lock); 1613 + svc_sock_put(svsk); 1614 } 1615 1616 /*