tree 45b22ab1088fb56f0cdc2afb8e8d0f071b5ad9f3
parent 0622f02b5aac4ae104f809a6beb836ccecef9a60
author Willy Tarreau <w@1wt.eu> 1492031011 +0200
committer Willy Tarreau <w@1wt.eu> 1492078852 +0200
encoding latin1

BUG/MEDIUM: acl: don't free unresolved args in prune_acl_expr()

In case of error it's very difficult to properly unroll the list of
unresolved args because the error can appear on any argument, and all
of them share the same memory area, pointed to by one or multiple links
from the global args list. The problem is that till now the arguments
themselves were released and were not unlinked from the list, causing
all forms of corruption in deinit() when quitting on the error path if
an argument couldn't properly parse.

A few attempts at trying to selectively spot the appropriate list entries
to kill before releasing the shared area have only resulted in complicating
the code and pushing the issue further.

Here instead we use a simple conservative approach : prune_acl_expr()
only tries to free the argument array if none of the arguments were
unresolved, which means that none of them was added to the arg list.

It's unclear what a better approach would be. We could imagine that
args would point to their own location in the shared list but given
that this extra cost and complexity would be added exclusively in
order to cleanly release everything when we're exiting due to a config
parse error, this seems quite overkill.

This bug was noticed on 1.7 and likely affects 1.6 and 1.5, so the fix
should be backported. It's not easy to reproduce it, as the reproducers
randomly work depending on how memory is allocated. One way to do it is
to use parsable and non-parsable patterns on an ACL making use of args.

Big thanks to Stephan Zeisberg for reporting this problem with a working
reproducer.