tlskeys_reference was a raw `struct list` instead of a `struct tls_keys_ref`
(which contains the `struct list`). This was uncritical in this specific case,
because the `struct list` is the first member of `struct tls_keys_ref`. Thus
address of the `struct tls_keys_ref` will be identical to the pointer of the
embedded `struct list`, resulting in the correct address / pointer being
returned by `LIST_ELEM()`.
However a reordering of the struct members might cause `LIST_ELEM` to return a
pointer to the left of an allocation, leading to undefined behavior.
This bug was found by gcc 11's `-Werror=array-bounds`, it was reported in
GitHub issue #1010.
This bug exists since `tlskeys_reference` exists. It was introduced in commit
200b0facdee1c033d28894b7822b2652f8fc6cd0. This commit first appeared in HAProxy
1.6-dev2. The fix should be backported to 1.6+.
---
include/haproxy/ssl_sock.h | 2 +-
src/cfgparse-ssl.c | 2 +-
src/ssl_sock.c | 24 +++++++++++++-----------
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
index ebfdb19ab..d5e7c27f1 100644
--- a/include/haproxy/ssl_sock.h
+++ b/include/haproxy/ssl_sock.h
@@ -31,7 +31,7 @@
#include <haproxy/ssl_sock-t.h>
#include <haproxy/thread.h>
-extern struct list tlskeys_reference;
+extern struct tls_keys_ref tlskeys_reference;
extern int sslconns;
extern int totalsslconns;
extern struct eb_root ckchs_tree;
diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index 3d58cd22d..189addbd1 100644
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -1182,7 +1182,7 @@ static int bind_parse_tls_ticket_keys(char **args, int
cur_arg, struct proxy *px
HA_RWLOCK_INIT(&keys_ref->lock);
conf->keys_ref = keys_ref;
- LIST_ADD(&tlskeys_reference, &keys_ref->list);
+ LIST_ADD(&tlskeys_reference.list, &keys_ref->list);
return 0;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 35298d530..01a5b3c4c 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -487,7 +487,9 @@ struct pool_head *pool_head_ssl_keylog_str = NULL;
#endif
#if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
-struct list tlskeys_reference = LIST_HEAD_INIT(tlskeys_reference);
+struct tls_keys_ref tlskeys_reference = {
+ .list = LIST_HEAD_INIT(tlskeys_reference.list)
+};
#endif
#ifndef OPENSSL_NO_ENGINE
@@ -1136,7 +1138,7 @@ struct tls_keys_ref *tlskeys_ref_lookup(const char
*filename)
{
struct tls_keys_ref *ref;
- list_for_each_entry(ref, &tlskeys_reference, list)
+ list_for_each_entry(ref, &tlskeys_reference.list, list)
if (ref->filename && strcmp(filename, ref->filename) == 0)
return ref;
return NULL;
@@ -1146,7 +1148,7 @@ struct tls_keys_ref *tlskeys_ref_lookupid(int unique_id)
{
struct tls_keys_ref *ref;
- list_for_each_entry(ref, &tlskeys_reference, list)
+ list_for_each_entry(ref, &tlskeys_reference.list, list)
if (ref->unique_id == unique_id)
return ref;
return NULL;
@@ -1205,17 +1207,17 @@ static int tlskeys_finalize_config(void)
struct tls_keys_ref *ref, *ref2, *ref3;
struct list tkr = LIST_HEAD_INIT(tkr);
- list_for_each_entry(ref, &tlskeys_reference, list) {
+ list_for_each_entry(ref, &tlskeys_reference.list, list) {
if (ref->unique_id == -1) {
/* Look for the first free id. */
while (1) {
- list_for_each_entry(ref2, &tlskeys_reference,
list) {
+ list_for_each_entry(ref2,
&tlskeys_reference.list, list) {
if (ref2->unique_id == i) {
i++;
break;
}
}
- if (&ref2->list == &tlskeys_reference)
+ if (&ref2->list == &tlskeys_reference.list)
break;
}
@@ -1226,7 +1228,7 @@ static int tlskeys_finalize_config(void)
}
/* This sort the reference list by id. */
- list_for_each_entry_safe(ref, ref2, &tlskeys_reference, list) {
+ list_for_each_entry_safe(ref, ref2, &tlskeys_reference.list, list) {
LIST_DEL(&ref->list);
list_for_each_entry(ref3, &tkr, list) {
if (ref->unique_id < ref3->unique_id) {
@@ -1239,7 +1241,7 @@ static int tlskeys_finalize_config(void)
}
/* swap root */
- LIST_ADD(&tkr, &tlskeys_reference);
+ LIST_ADD(&tkr, &tlskeys_reference.list);
LIST_DEL(&tkr);
return ERR_NONE;
}
@@ -6435,8 +6437,8 @@ static int cli_io_handler_tlskeys_files(struct appctx
*appctx) {
* tlskeys_list_get_next() for retruning the first available
entry
*/
if (appctx->ctx.cli.p0 == NULL) {
- appctx->ctx.cli.p0 = LIST_ELEM(&tlskeys_reference,
struct tls_keys_ref *, list);
- appctx->ctx.cli.p0 =
tlskeys_list_get_next(appctx->ctx.cli.p0, &tlskeys_reference);
+ appctx->ctx.cli.p0 = LIST_ELEM(&tlskeys_reference.list,
struct tls_keys_ref *, list);
+ appctx->ctx.cli.p0 =
tlskeys_list_get_next(appctx->ctx.cli.p0, &tlskeys_reference.list);
}
appctx->st2 = STAT_ST_LIST;
@@ -6507,7 +6509,7 @@ static int cli_io_handler_tlskeys_files(struct appctx
*appctx) {
break;
/* get next list entry and check the end of the list */
- appctx->ctx.cli.p0 =
tlskeys_list_get_next(appctx->ctx.cli.p0, &tlskeys_reference);
+ appctx->ctx.cli.p0 =
tlskeys_list_get_next(appctx->ctx.cli.p0, &tlskeys_reference.list);
}
appctx->st2 = STAT_ST_FIN;
--
2.29.0
