[PATCH] kNFSd: gss_svc locking and refcounting fixes

The server sunrpc code should take a reference on the relevant module before
calling any authentication code.

Also, it looks to me like the table of authops needs some locking.

Finally, gss_svc_init wasn't checking the status of svc_auth_register, and
gss_svc_shutdown wasn't calling svc_auth_unregister.

From: "J. Bruce Fields" <bfields@fieldses.org>
Signed-off-by: Neil Brown <neilb@cse.unsw.edu.au>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index 36bcc5ed72d01a23fe9f953c9d547f925dd49751..30689cc5734f6c1304d49bf5bae4fe9c040942a9 100644 (file)
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -87,6 +87,7 @@ struct auth_domain {
  */
 struct auth_ops {
        char *  name;
+       struct module *owner;
        int     flavour;
        int     (*accept)(struct svc_rqst *rq, u32 *authp);
        int     (*release)(struct svc_rqst *rq);

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 03bd7bd8c7d8010e4c43c21049e4d298da963572..bc2f949af2b78d276c8e03bb2734964bb6f4886b 100644 (file)
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1045,6 +1045,7 @@ svcauth_gss_domain_release(struct auth_domain *dom)
 
 struct auth_ops svcauthops_gss = {
        .name           = "rpcsec_gss",
+       .owner          = THIS_MODULE,
        .flavour        = RPC_AUTH_GSS,
        .accept         = svcauth_gss_accept,
        .release        = svcauth_gss_release,
@@ -1054,10 +1055,12 @@ struct auth_ops svcauthops_gss = {
 int
 gss_svc_init(void)
 {
-       cache_register(&rsc_cache);
-       cache_register(&rsi_cache);
-       svc_auth_register(RPC_AUTH_GSS, &svcauthops_gss);
-       return 0;
+       int rv = svc_auth_register(RPC_AUTH_GSS, &svcauthops_gss);
+       if (rv == 0) {
+               cache_register(&rsc_cache);
+               cache_register(&rsi_cache);
+       }
+       return rv;
 }
 
 void
@@ -1065,4 +1068,5 @@ gss_svc_shutdown(void)
 {
        cache_unregister(&rsc_cache);
        cache_unregister(&rsi_cache);
+       svc_auth_unregister(RPC_AUTH_GSS);
 }

diff --git a/net/sunrpc/svcauth.c b/net/sunrpc/svcauth.c
index db78e727d1ebf429a810435fd23941c6527f1c8e..54ea896296286d6f42de9f4a3e2f4433b0ea6262 100644 (file)
--- a/net/sunrpc/svcauth.c
+++ b/net/sunrpc/svcauth.c
@@ -11,6 +11,7 @@
 
 #include <linux/types.h>
 #include <linux/sched.h>
+#include <linux/module.h>
 #include <linux/sunrpc/types.h>
 #include <linux/sunrpc/xdr.h>
 #include <linux/sunrpc/svcsock.h>
@@ -27,6 +28,7 @@
 extern struct auth_ops svcauth_null;
 extern struct auth_ops svcauth_unix;
 
+static spinlock_t authtab_lock = SPIN_LOCK_UNLOCKED;
 static struct auth_ops *authtab[RPC_AUTH_MAXFLAVOR] = {
        [0] = &svcauth_null,
        [1] = &svcauth_unix,
@@ -43,10 +45,15 @@ svc_authenticate(struct svc_rqst *rqstp, u32 *authp)
        flavor = ntohl(svc_getu32(&rqstp->rq_arg.head[0]));
 
        dprintk("svc: svc_authenticate (%d)\n", flavor);
-       if (flavor >= RPC_AUTH_MAXFLAVOR || !(aops = authtab[flavor])) {
+
+       spin_lock(&authtab_lock);
+       if (flavor >= RPC_AUTH_MAXFLAVOR || !(aops = authtab[flavor])
+                       || !try_module_get(aops->owner)) {
+               spin_unlock(&authtab_lock);
                *authp = rpc_autherr_badcred;
                return SVC_DENIED;
        }
+       spin_unlock(&authtab_lock);
 
        rqstp->rq_authop = aops;
        return aops->accept(rqstp, authp);
@@ -63,28 +70,35 @@ int svc_authorise(struct svc_rqst *rqstp)
 
        rqstp->rq_authop = NULL;
        
-       if (aops) 
+       if (aops) {
                rv = aops->release(rqstp);
-
-       /* FIXME should I count and release authops */
+               module_put(aops->owner);
+       }
        return rv;
 }
 
 int
 svc_auth_register(rpc_authflavor_t flavor, struct auth_ops *aops)
 {
-       if (flavor >= RPC_AUTH_MAXFLAVOR || authtab[flavor])
-               return -EINVAL;
-       authtab[flavor] = aops;
-       return 0;
+       int rv = -EINVAL;
+       spin_lock(&authtab_lock);
+       if (flavor < RPC_AUTH_MAXFLAVOR && authtab[flavor] == NULL) {
+               authtab[flavor] = aops;
+               rv = 0;
+       }
+       spin_unlock(&authtab_lock);
+       return rv;
 }
 
 void
 svc_auth_unregister(rpc_authflavor_t flavor)
 {
+       spin_lock(&authtab_lock);
        if (flavor < RPC_AUTH_MAXFLAVOR)
                authtab[flavor] = NULL;
+       spin_unlock(&authtab_lock);
 }
+EXPORT_SYMBOL(svc_auth_unregister);
 
 /**************************************************
  * cache for domain name to auth_domain

diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 082684ddf3bc828a046f43ba548d5a1bc06477b4..ab5164b6dab334c41a8a4520923a775a30e5da36 100644 (file)
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -1,5 +1,6 @@
 #include <linux/types.h>
 #include <linux/sched.h>
+#include <linux/module.h>
 #include <linux/sunrpc/types.h>
 #include <linux/sunrpc/xdr.h>
 #include <linux/sunrpc/svcsock.h>
@@ -411,6 +412,7 @@ svcauth_null_release(struct svc_rqst *rqstp)
 
 struct auth_ops svcauth_null = {
        .name           = "null",
+       .owner          = THIS_MODULE,
        .flavour        = RPC_AUTH_NULL,
        .accept         = svcauth_null_accept,
        .release        = svcauth_null_release,
@@ -515,6 +517,7 @@ svcauth_unix_release(struct svc_rqst *rqstp)
 
 struct auth_ops svcauth_unix = {
        .name           = "unix",
+       .owner          = THIS_MODULE,
        .flavour        = RPC_AUTH_UNIX,
        .accept         = svcauth_unix_accept,
        .release        = svcauth_unix_release,