From c138631cd16e5a28cf7f5169bee28ed4c6dae598 Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Sat, 22 Mar 2014 18:56:28 -0400 Subject: [PATCH] Fix maxclients error handling Everywhere in the Redis code base, maxclients is treated as an int with (int)maxclients or `maxclients = atoi(source)`, so let's make maxclients an int. This fixes a bug where someone could specify a negative maxclients on startup and it would work (as well as set maxclients very high) because: unsigned int maxclients; char *update = "-300"; maxclients = atoi(update); if (maxclients < 1) goto fail; But, (maxclients < 1) can only catch the case when maxclients is exactly 0. maxclients happily sets itself to -300, which isn't -300, but rather 4294966996, which isn't < 1, so... everything "worked." maxclients config parsing checks for the case of < 1, but maxclients CONFIG SET parsing was checking for case of < 0 (allowing maxclients to be set to 0). CONFIG SET parsing is now updated to match config parsing of < 1. It's tempting to add a MINIMUM_CLIENTS define, but... I didn't. These changes were inspired by antirez#356, but this doesn't fix that issue. --- src/config.c | 2 +- src/redis.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.c b/src/config.c index 0d168c03f..15ea1c5c3 100644 --- a/src/config.c +++ b/src/config.c @@ -600,7 +600,7 @@ void configSetCommand(redisClient *c) { } else if (!strcasecmp(c->argv[2]->ptr,"maxclients")) { int orig_value = server.maxclients; - if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll < 0) goto badfmt; + if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll < 1) goto badfmt; /* Try to check if the OS is capable of supporting so many FDs. */ server.maxclients = ll; diff --git a/src/redis.h b/src/redis.h index fd5dac19c..5d57410e8 100644 --- a/src/redis.h +++ b/src/redis.h @@ -786,7 +786,7 @@ struct redisServer { list *clients_waiting_acks; /* Clients waiting in WAIT command. */ int get_ack_from_slaves; /* If true we send REPLCONF GETACK. */ /* Limits */ - unsigned int maxclients; /* Max number of simultaneous clients */ + int maxclients; /* Max number of simultaneous clients */ unsigned long long maxmemory; /* Max number of memory bytes to use */ int maxmemory_policy; /* Policy for key eviction */ int maxmemory_samples; /* Pricision of random sampling */