gamzinav/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MINOR: ssl: make tlskeys_list_get_next() take a list element

Browse Source 
As reported in issue #1010, gcc-11 as of 2021-01-05 is overzealous in
its -Warray-bounds check as it considers that a cast of a global struct
accesses the entire struct even if only one specific element is accessed.
This instantly breaks all lists making use of container_of() to build
their iterators as soon as the starting point is known if the next
element is retrieved from the list head in a way that is visible to the
compiler's optimizer, because it decides that accessing the list's next
element dereferences the list as a larger struct (which it does not).

The temporary workaround consisted in disabling -Warray-bounds, but this
warning is traditionally quite effective at spotting real bugs, and we
actually have is a single occurrence of this issue in the whole code.

By changing the tlskeys_list_get_next() function to take a list element
as the starting point instead of the current element, we can avoid
the starting point issue but this requires to change all call places
to write hideous casts made of &((struct blah*)ref)->list. At the
moment we only have two such call places, the first one being used to
initialize the list (which is the one causing the warning) and which
is thus easy to simplify, and the second one for which we already have
an aliased pointer to the reference that is still valid at the call
place, and given the original pointer also remained unchanged, we can
safely use this alias, and this is safer than leaving a cast there.

Let's make this change now while it's still easy.

The generated code only changed in function cli_io_handler_tlskeys_files()
due to register allocation and the change of variable scope between the
old one and the new one.
This commit is contained in:
Willy Tarreau 2021-01-05 10:44:30 +01:00
parent cb8b281c02
commit b6fc524f05
1 changed files with 11 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/ssl_sock.c
View File

@ -6350,22 +6350,22 @@ static int ssl_check_async_engine_count(void) {
}
#endif
/* This function is used with TLS ticket keys management. It permits to browse
* each reference. The variable <getnext> must contain the current node,
* <end> point to the root node.
*/
#if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
/* This function is used with TLS ticket keys management. It permits to browse
* each reference. The variable <ref> must point to the current node's list
* element (which starts by the root), and <end> must point to the root node.
*/
static inline
struct tls_keys_ref *tlskeys_list_get_next(struct tls_keys_ref *ref, struct list *end)
struct tls_keys_ref *tlskeys_list_get_next(struct list *ref, struct list *end)
{
/* Get next list entry. */
ref = LIST_NEXT(&ref->list, struct tls_keys_ref *, list);
ref = ref->n;
/* If the entry is the last of the list, return NULL. */
if (&ref->list == end)
if (ref == end)
return NULL;
return ref;
return LIST_ELEM(ref, struct tls_keys_ref *, list);
}
static inline
@ -6429,10 +6429,8 @@ static int cli_io_handler_tlskeys_files(struct appctx *appctx) {
* available field of this pointer is <list>. It is used with the function
* 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);
}
if (appctx->ctx.cli.p0 == NULL)
appctx->ctx.cli.p0 = tlskeys_list_get_next(&tlskeys_reference, &tlskeys_reference);
appctx->st2 = STAT_ST_LIST;
/* fall through */
@ -6502,7 +6500,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(&ref->list, &tlskeys_reference);
}
appctx->st2 = STAT_ST_FIN;