On Mon, 2005-08-29 at 16:45 -0700, Stephen Hemminger wrote:
> You must check return value from try_module_get and error out
> if it fails. This is critical for module unload races.

Good point, and of course in such a situation, sk_alloc must
free the socket. Please see the updated patch below.

Thanks

Frank

---

Signed-off-by: Frank Filz <[EMAIL PROTECTED]>

diff --git a/net/core/sock.c b/net/core/sock.c
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -660,16 +660,20 @@ struct sock *sk_alloc(int family, unsign
                        sock_lock_init(sk);
                }
                
-               if (security_sk_alloc(sk, family, priority)) {
-                       if (slab != NULL)
-                               kmem_cache_free(slab, sk);
-                       else
-                               kfree(sk);
-                       sk = NULL;
-               } else
-                       __module_get(prot->owner);
+               if (security_sk_alloc(sk, family, priority))
+                       goto out_free;
+
+               if (!try_module_get(prot->owner))
+                       goto out_free;
        }
        return sk;
+
+out_free:
+       if (slab != NULL)
+               kmem_cache_free(slab, sk);
+       else
+               kfree(sk);
+       return NULL;
 }
 
 void sk_free(struct sock *sk)
diff --git a/net/socket.c b/net/socket.c
--- a/net/socket.c
+++ b/net/socket.c
@@ -1145,8 +1145,11 @@ static int __sock_create(int family, int
        if (!try_module_get(net_families[family]->owner))
                goto out_release;
 
-       if ((err = net_families[family]->create(sock, protocol)) < 0)
+       if ((err = net_families[family]->create(sock, protocol)) < 0) {
+               sock->ops = NULL;
                goto out_module_put;
+       }
+
        /*
         * Now to bump the refcnt of the [loadable] module that owns this
         * socket at sock_release time we decrement its refcnt.
@@ -1360,16 +1363,16 @@ asmlinkage long sys_accept(int fd, struc
        newsock->type = sock->type;
        newsock->ops = sock->ops;
 
-       err = security_socket_accept(sock, newsock);
-       if (err)
-               goto out_release;
-
        /*
         * We don't need try_module_get here, as the listening socket (sock)
         * has the protocol module (sock->ops->owner) held.
         */
        __module_get(newsock->ops->owner);
 
+       err = security_socket_accept(sock, newsock);
+       if (err)
+               goto out_release;
+
        err = sock->ops->accept(sock, newsock, sock->file->f_flags);
        if (err < 0)
                goto out_release;



-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to