svc_check_conn_limits() attempts to prevent denial of service attacks
by having the service close old connections once it reaches a
threshold. This threshold is based on the number of threads in the
service:
(serv->sv_nrthreads + 3) * 20
Once we reach this, we drop the oldest connections and a printk pops
to warn the admin that they should increase the number of threads.
Increasing the number of threads isn't an option however for services
like lockd. We don't want to eliminate this check entirely for such
services but we need some way to increase this limit.
This patch adds a sv_maxconn field to the svc_serv struct. When it's
set to 0, we use the current method to calculate the max number of
connections. RPC services can then set this on an as-needed basis.
Signed-off-by: Jeff Layton <[email protected]>
---
include/linux/sunrpc/svc.h | 5 ++++-
net/sunrpc/svc_xprt.c | 22 ++++++++++++++++------
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 3afe7fb..3435d24 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -58,10 +58,13 @@ struct svc_serv {
struct svc_stat * sv_stats; /* RPC statistics */
spinlock_t sv_lock;
unsigned int sv_nrthreads; /* # of server threads */
+ unsigned int sv_maxconn; /* max connections allowed or
+ * '0' causing max to be based
+ * on number of threads. */
+
unsigned int sv_max_payload; /* datagram payload size */
unsigned int sv_max_mesg; /* max_payload + 1 page for overheads */
unsigned int sv_xdrsize; /* XDR buffer size */
-
struct list_head sv_permsocks; /* all permanent sockets */
struct list_head sv_tempsocks; /* all temporary sockets */
int sv_tmpcnt; /* count of temporary sockets */
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index bf5b5cd..3fe4f10 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -515,8 +515,10 @@ int svc_port_is_privileged(struct sockaddr *sin)
}
/*
- * Make sure that we don't have too many active connections. If we
- * have, something must be dropped.
+ * Make sure that we don't have too many active connections. If we have,
+ * something must be dropped. It's not clear what will happen if we allow
+ * "too many" connections, but when dealing with network-facing software,
+ * we have to code defensively. Here we do that by imposing hard limits.
*
* There's no point in trying to do random drop here for DoS
* prevention. The NFS clients does 1 reconnect in 15 seconds. An
@@ -525,19 +527,27 @@ int svc_port_is_privileged(struct sockaddr *sin)
* The only somewhat efficient mechanism would be if drop old
* connections from the same IP first. But right now we don't even
* record the client IP in svc_sock.
+ *
+ * single-threaded services that expect a lot of clients will probably
+ * need to set sv_maxconn to override the default value which is based
+ * on the number of threads
*/
static void svc_check_conn_limits(struct svc_serv *serv)
{
- if (serv->sv_tmpcnt > (serv->sv_nrthreads+3)*20) {
+ unsigned int limit = serv->sv_maxconn ? serv->sv_maxconn :
+ (serv->sv_nrthreads+3) * 20;
+
+ if (serv->sv_tmpcnt > limit) {
struct svc_xprt *xprt = NULL;
spin_lock_bh(&serv->sv_lock);
if (!list_empty(&serv->sv_tempsocks)) {
if (net_ratelimit()) {
/* Try to help the admin */
printk(KERN_NOTICE "%s: too many open "
- "connections, consider increasing the "
- "number of nfsd threads\n",
- serv->sv_name);
+ "connections, consider increasing %s\n",
+ serv->sv_name, serv->sv_maxconn ?
+ "the max number of connections." :
+ "the number of threads.");
}
/*
* Always select the oldest connection. It's not fair,
--
1.5.5.1
The default method for calculating the number of connections allowed
per RPC service arbitrarily limits single-threaded services to 80
connections. This is too low for services like lockd and artificially
limits the number of TCP clients that it can support.
Have lockd set a default sv_maxconn value to 1024 (which is the typical
default value for RLIMIT_NOFILE. Also add a module parameter to allow an
admin to set this to an arbitrary value.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/lockd/svc.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index c631a83..e804155 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -62,6 +62,9 @@ static unsigned long nlm_timeout = LOCKD_DFLT_TIMEO;
static int nlm_udpport, nlm_tcpport;
int nsm_use_hostnames = 0;
+/* RLIM_NOFILE defaults to 1024. That seems like a reasonable default here. */
+static unsigned int nlm_max_connections = 1024;
+
/*
* Constants needed for the sysctl interface.
*/
@@ -143,6 +146,9 @@ lockd(void *vrqstp)
long timeout = MAX_SCHEDULE_TIMEOUT;
RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ /* update sv_maxconn if it has changed */
+ rqstp->rq_server->sv_maxconn = nlm_max_connections;
+
if (signalled()) {
flush_signals(current);
if (nlmsvc_ops) {
@@ -275,6 +281,7 @@ int lockd_up(void)
}
svc_sock_update_bufs(serv);
+ serv->sv_maxconn = nlm_max_connections;
nlmsvc_task = kthread_run(lockd, nlmsvc_rqst, serv->sv_name);
if (IS_ERR(nlmsvc_task)) {
@@ -484,6 +491,7 @@ module_param_call(nlm_udpport, param_set_port, param_get_int,
module_param_call(nlm_tcpport, param_set_port, param_get_int,
&nlm_tcpport, 0644);
module_param(nsm_use_hostnames, bool, 0644);
+module_param(nlm_max_connections, uint, 0644);
/*
* Initialising and terminating the module.
--
1.5.5.1
My one small worry is that we'll find a better fix for all this at some
point, and then the sysctl will be a useless dangling appendage. But,
OK--both applied.
--b.
On Mon, Oct 20, 2008 at 11:51:57AM -0400, Jeff Layton wrote:
> svc_check_conn_limits() attempts to prevent denial of service attacks
> by having the service close old connections once it reaches a
> threshold. This threshold is based on the number of threads in the
> service:
>
> (serv->sv_nrthreads + 3) * 20
>
> Once we reach this, we drop the oldest connections and a printk pops
> to warn the admin that they should increase the number of threads.
>
> Increasing the number of threads isn't an option however for services
> like lockd. We don't want to eliminate this check entirely for such
> services but we need some way to increase this limit.
>
> This patch adds a sv_maxconn field to the svc_serv struct. When it's
> set to 0, we use the current method to calculate the max number of
> connections. RPC services can then set this on an as-needed basis.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> include/linux/sunrpc/svc.h | 5 ++++-
> net/sunrpc/svc_xprt.c | 22 ++++++++++++++++------
> 2 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 3afe7fb..3435d24 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -58,10 +58,13 @@ struct svc_serv {
> struct svc_stat * sv_stats; /* RPC statistics */
> spinlock_t sv_lock;
> unsigned int sv_nrthreads; /* # of server threads */
> + unsigned int sv_maxconn; /* max connections allowed or
> + * '0' causing max to be based
> + * on number of threads. */
> +
> unsigned int sv_max_payload; /* datagram payload size */
> unsigned int sv_max_mesg; /* max_payload + 1 page for overheads */
> unsigned int sv_xdrsize; /* XDR buffer size */
> -
> struct list_head sv_permsocks; /* all permanent sockets */
> struct list_head sv_tempsocks; /* all temporary sockets */
> int sv_tmpcnt; /* count of temporary sockets */
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index bf5b5cd..3fe4f10 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -515,8 +515,10 @@ int svc_port_is_privileged(struct sockaddr *sin)
> }
>
> /*
> - * Make sure that we don't have too many active connections. If we
> - * have, something must be dropped.
> + * Make sure that we don't have too many active connections. If we have,
> + * something must be dropped. It's not clear what will happen if we allow
> + * "too many" connections, but when dealing with network-facing software,
> + * we have to code defensively. Here we do that by imposing hard limits.
> *
> * There's no point in trying to do random drop here for DoS
> * prevention. The NFS clients does 1 reconnect in 15 seconds. An
> @@ -525,19 +527,27 @@ int svc_port_is_privileged(struct sockaddr *sin)
> * The only somewhat efficient mechanism would be if drop old
> * connections from the same IP first. But right now we don't even
> * record the client IP in svc_sock.
> + *
> + * single-threaded services that expect a lot of clients will probably
> + * need to set sv_maxconn to override the default value which is based
> + * on the number of threads
> */
> static void svc_check_conn_limits(struct svc_serv *serv)
> {
> - if (serv->sv_tmpcnt > (serv->sv_nrthreads+3)*20) {
> + unsigned int limit = serv->sv_maxconn ? serv->sv_maxconn :
> + (serv->sv_nrthreads+3) * 20;
> +
> + if (serv->sv_tmpcnt > limit) {
> struct svc_xprt *xprt = NULL;
> spin_lock_bh(&serv->sv_lock);
> if (!list_empty(&serv->sv_tempsocks)) {
> if (net_ratelimit()) {
> /* Try to help the admin */
> printk(KERN_NOTICE "%s: too many open "
> - "connections, consider increasing the "
> - "number of nfsd threads\n",
> - serv->sv_name);
> + "connections, consider increasing %s\n",
> + serv->sv_name, serv->sv_maxconn ?
> + "the max number of connections." :
> + "the number of threads.");
> }
> /*
> * Always select the oldest connection. It's not fair,
> --
> 1.5.5.1
>
On Mon, 20 Oct 2008 18:51:58 -0400
"J. Bruce Fields" <[email protected]> wrote:
> My one small worry is that we'll find a better fix for all this at some
> point, and then the sysctl will be a useless dangling appendage. But,
> OK--both applied.
>
Thanks, Bruce. If we can think of a better way to do this, I'm all for
it. Hopefully it won't be too difficult a thing to change if that
happens.
--
Jeff Layton <[email protected]>