Commit a79af59e authored by Frank Filz's avatar Frank Filz Committed by David S. Miller

[NET]: Fix module reference counts for loadable protocol modules

I have been experimenting with loadable protocol modules, and ran into
several issues with module reference counting.

The first issue was that __module_get failed at the BUG_ON check at
the top of the routine (checking that my module reference count was
not zero) when I created the first socket. When sk_alloc() is called,
my module reference count was still 0. When I looked at why sctp
didn't have this problem, I discovered that sctp creates a control
socket during module init (when the module ref count is not 0), which
keeps the reference count non-zero. This section has been updated to
address the point Stephen raised about checking the return value of
try_module_get().

The next problem arose when my socket init routine returned an error.
This resulted in my module reference count being decremented below 0.
My socket ops->release routine was also being called. The issue here
is that sock_release() calls the ops->release routine and decrements
the ref count if sock->ops is not NULL. Since the socket probably
didn't get correctly initialized, this should not be done, so we will
set sock->ops to NULL because we will not call try_module_get().

While searching for another bug, I also noticed that sys_accept() has
a possibility of doing a module_put() when it did not do an
__module_get so I re-ordered the call to security_socket_accept().
Signed-off-by: default avatarFrank Filz <ffilzlnx@us.ibm.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 9356b8fc
...@@ -660,16 +660,20 @@ struct sock *sk_alloc(int family, unsigned int __nocast priority, ...@@ -660,16 +660,20 @@ struct sock *sk_alloc(int family, unsigned int __nocast priority,
sock_lock_init(sk); sock_lock_init(sk);
} }
if (security_sk_alloc(sk, family, priority)) { 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) if (slab != NULL)
kmem_cache_free(slab, sk); kmem_cache_free(slab, sk);
else else
kfree(sk); kfree(sk);
sk = NULL; return NULL;
} else
__module_get(prot->owner);
}
return sk;
} }
void sk_free(struct sock *sk) void sk_free(struct sock *sk)
......
...@@ -1145,8 +1145,11 @@ static int __sock_create(int family, int type, int protocol, struct socket **res ...@@ -1145,8 +1145,11 @@ static int __sock_create(int family, int type, int protocol, struct socket **res
if (!try_module_get(net_families[family]->owner)) if (!try_module_get(net_families[family]->owner))
goto out_release; 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; goto out_module_put;
}
/* /*
* Now to bump the refcnt of the [loadable] module that owns this * Now to bump the refcnt of the [loadable] module that owns this
* socket at sock_release time we decrement its refcnt. * socket at sock_release time we decrement its refcnt.
...@@ -1360,16 +1363,16 @@ asmlinkage long sys_accept(int fd, struct sockaddr __user *upeer_sockaddr, int _ ...@@ -1360,16 +1363,16 @@ asmlinkage long sys_accept(int fd, struct sockaddr __user *upeer_sockaddr, int _
newsock->type = sock->type; newsock->type = sock->type;
newsock->ops = sock->ops; 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) * We don't need try_module_get here, as the listening socket (sock)
* has the protocol module (sock->ops->owner) held. * has the protocol module (sock->ops->owner) held.
*/ */
__module_get(newsock->ops->owner); __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); err = sock->ops->accept(sock, newsock, sock->file->f_flags);
if (err < 0) if (err < 0)
goto out_release; goto out_release;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment