From: Willy Tarreau Date: Mon, 29 Oct 2012 20:56:59 +0000 (+0100) Subject: MEDIUM: stick-table: allocate the table key of size buffer size X-Git-Tag: v1.5-dev13~85 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=07115412d3b7bb4c55e4fa5a88fdcf5450cb7a2f;p=thirdparty%2Fhaproxy.git MEDIUM: stick-table: allocate the table key of size buffer size Keys are copied from samples to stick_table_key. If a key is larger than the stick_table_key, we have an overflow. In pratice it does not happen because it requires : 1) a configuration with tune.bufsize larger than BUFSIZE (common) 2) a stick-table configured with keys strictly larger than buffers 3) extraction of data larger than BUFSIZE (eg: using payload()) Points 2 and 3 don't make any sense for a real world configuration. That said the issue needs be fixed. The solution consists in allocating it the same size as the global buffer size, just like the samples. This fixes the issue. --- diff --git a/include/proto/proto_tcp.h b/include/proto/proto_tcp.h index 1a1dcfab51..a41e025a87 100644 --- a/include/proto/proto_tcp.h +++ b/include/proto/proto_tcp.h @@ -48,15 +48,15 @@ static inline struct stktable_key *addr_to_stktable_key(struct sockaddr_storage { switch (addr->ss_family) { case AF_INET: - static_table_key.key = (void *)&((struct sockaddr_in *)addr)->sin_addr; + static_table_key->key = (void *)&((struct sockaddr_in *)addr)->sin_addr; break; case AF_INET6: - static_table_key.key = (void *)&((struct sockaddr_in6 *)addr)->sin6_addr; + static_table_key->key = (void *)&((struct sockaddr_in6 *)addr)->sin6_addr; break; default: return NULL; } - return &static_table_key; + return static_table_key; } diff --git a/include/proto/stick_table.h b/include/proto/stick_table.h index f7d650a157..d9a25d0c7d 100644 --- a/include/proto/stick_table.h +++ b/include/proto/stick_table.h @@ -31,7 +31,7 @@ #define stktable_data_size(type) (sizeof(((union stktable_data*)0)->type)) #define stktable_data_cast(ptr, type) ((union stktable_data*)(ptr))->type -extern struct stktable_key static_table_key; +extern struct stktable_key *static_table_key; struct stksess *stksess_new(struct stktable *t, struct stktable_key *key); void stksess_setkey(struct stktable *t, struct stksess *ts, struct stktable_key *key); diff --git a/include/types/stick_table.h b/include/types/stick_table.h index cccf9fbecd..520f57aed0 100644 --- a/include/types/stick_table.h +++ b/include/types/stick_table.h @@ -176,15 +176,14 @@ union stktable_key_data { struct in_addr ip; /* used to store an ipv4 key */ struct in6_addr ipv6; /* used to store an ipv6 key */ uint32_t integer; /* used to store an integer key */ - char buf[BUFSIZE]; /* used to store a null terminated string key or a buffer of data */ + char buf[0]; /* dynamically allocated, used to store a null terminated string key or a buffer of data */ }; /* stick table key */ struct stktable_key { void *key; /* pointer on key buffer */ size_t key_len; /* data len to read in buff in case of null terminated string */ - union stktable_key_data data; /* data */ + union stktable_key_data data; /* data, must always be last */ }; #endif /* _TYPES_STICK_TABLE_H */ - diff --git a/src/dumpstats.c b/src/dumpstats.c index eefce5812c..da84f2a3de 100644 --- a/src/dumpstats.c +++ b/src/dumpstats.c @@ -541,11 +541,11 @@ static void stats_sock_table_key_request(struct stream_interface *si, char **arg switch (px->table.type) { case STKTABLE_TYPE_IP: uint32_key = htonl(inetaddr_host(args[4])); - static_table_key.key = &uint32_key; + static_table_key->key = &uint32_key; break; case STKTABLE_TYPE_IPV6: inet_pton(AF_INET6, args[4], ip6_key); - static_table_key.key = &ip6_key; + static_table_key->key = &ip6_key; break; case STKTABLE_TYPE_INTEGER: { @@ -561,13 +561,13 @@ static void stats_sock_table_key_request(struct stream_interface *si, char **arg return; } uint32_key = (uint32_t) val; - static_table_key.key = &uint32_key; + static_table_key->key = &uint32_key; break; } break; case STKTABLE_TYPE_STRING: - static_table_key.key = args[4]; - static_table_key.key_len = strlen(args[4]); + static_table_key->key = args[4]; + static_table_key->key_len = strlen(args[4]); break; default: switch (action) { @@ -592,7 +592,7 @@ static void stats_sock_table_key_request(struct stream_interface *si, char **arg return; } - ts = stktable_lookup_key(&px->table, &static_table_key); + ts = stktable_lookup_key(&px->table, static_table_key); switch (action) { case STAT_CLI_O_TAB: @@ -645,7 +645,7 @@ static void stats_sock_table_key_request(struct stream_interface *si, char **arg if (ts) stktable_touch(&px->table, ts, 1); else { - ts = stksess_new(&px->table, &static_table_key); + ts = stksess_new(&px->table, static_table_key); if (!ts) { /* don't delete an entry which is currently referenced */ si->applet.ctx.cli.msg = "Unable to allocate a new entry\n"; diff --git a/src/haproxy.c b/src/haproxy.c index 068a3ddf1d..c6933c33d4 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -745,6 +745,7 @@ void init(int argc, char **argv) sample_trash_buf1 = (char *)calloc(1, global.tune.bufsize); sample_trash_buf2 = (char *)calloc(1, global.tune.bufsize); get_http_auth_buff = (char *)calloc(1, global.tune.bufsize); + static_table_key = calloc(1, sizeof(*static_table_key) + global.tune.bufsize); fdinfo = (struct fdinfo *)calloc(1, diff --git a/src/stick_table.c b/src/stick_table.c index 75267e1d38..b681a8b86b 100644 --- a/src/stick_table.c +++ b/src/stick_table.c @@ -31,7 +31,7 @@ #include /* structure used to return a table key built from a sample */ -struct stktable_key static_table_key; +struct stktable_key *static_table_key; /* * Free an allocated sticky session , and decrease sticky sessions counter @@ -618,42 +618,42 @@ struct stktable_key *stktable_fetch_key(struct stktable *t, struct proxy *px, st if (!sample_to_key[smp->type][t->type]) return NULL; - static_table_key.key_len = t->key_size; - static_table_key.key = sample_to_key[smp->type][t->type](smp, &static_table_key.data, &static_table_key.key_len); + static_table_key->key_len = t->key_size; + static_table_key->key = sample_to_key[smp->type][t->type](smp, &static_table_key->data, &static_table_key->key_len); - if (!static_table_key.key) + if (!static_table_key->key) return NULL; - if (static_table_key.key_len == 0) + if (static_table_key->key_len == 0) return NULL; - if ((static_table_key.key_len < t->key_size) && (t->type != STKTABLE_TYPE_STRING)) { + if ((static_table_key->key_len < t->key_size) && (t->type != STKTABLE_TYPE_STRING)) { /* need padding with null */ /* assume static_table_key.key_len is less than sizeof(static_table_key.data.buf) cause t->key_size is necessary less than sizeof(static_table_key.data) */ - if ((char *)static_table_key.key > (char *)&static_table_key.data && - (char *)static_table_key.key < (char *)&static_table_key.data + sizeof(static_table_key.data)) { + if ((char *)static_table_key->key > (char *)&static_table_key->data && + (char *)static_table_key->key < (char *)&static_table_key->data + global.tune.bufsize) { /* key buffer is part of the static_table_key private data buffer, but is not aligned */ - if (sizeof(static_table_key.data) - ((char *)static_table_key.key - (char *)&static_table_key.data) < t->key_size) { - /* if not remain enougth place for padding , process a realign */ - memmove(static_table_key.data.buf, static_table_key.key, static_table_key.key_len); - static_table_key.key = static_table_key.data.buf; + if (global.tune.bufsize - ((char *)static_table_key->key - (char *)&static_table_key->data) < t->key_size) { + /* if not remain enough place for padding , process a realign */ + memmove(static_table_key->data.buf, static_table_key->key, static_table_key->key_len); + static_table_key->key = static_table_key->data.buf; } } - else if (static_table_key.key != static_table_key.data.buf) { + else if (static_table_key->key != static_table_key->data.buf) { /* key definitly not part of the static_table_key private data buffer */ - memcpy(static_table_key.data.buf, static_table_key.key, static_table_key.key_len); - static_table_key.key = static_table_key.data.buf; + memcpy(static_table_key->data.buf, static_table_key->key, static_table_key->key_len); + static_table_key->key = static_table_key->data.buf; } - memset(static_table_key.key + static_table_key.key_len, 0, t->key_size - static_table_key.key_len); + memset(static_table_key->key + static_table_key->key_len, 0, t->key_size - static_table_key->key_len); } - return &static_table_key; + return static_table_key; } /*