Detecting use-after-free bugs using gcc's malloc() attribute

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Detecting use-after-free bugs using gcc's malloc() attribute
Date: 2023-06-26 19:54:44
Message-ID: 20230626195444.vtbcotrxj65cyqvq@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I played around with adding
__attribute__((malloc(free_func), malloc(another_free_func)))
annotations to a few functions in pg. See
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

Adding them to pg_list.h seems to have found two valid issues when compiling
without optimization:

[1001/2331 22 42%] Compiling C object src/backend/postgres_lib.a.p/commands_tablecmds.c.o
../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c:17966:25: warning: pointer ‘partBoundConstraint’ may be used after ‘list_concat’ [-Wuse-after-free]
17966 | get_proposed_default_constraint(partBoundConstraint);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c:17919:26: note: call to ‘list_concat’ here
17919 | partConstraint = list_concat(partBoundConstraint,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17920 | RelationGetPartitionQual(rel));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[1233/2331 22 52%] Compiling C object src/backend/postgres_lib.a.p/rewrite_rewriteHandler.c.o
../../../../home/andres/src/postgresql/src/backend/rewrite/rewriteHandler.c: In function ‘rewriteRuleAction’:
../../../../home/andres/src/postgresql/src/backend/rewrite/rewriteHandler.c:550:41: warning: pointer ‘newjointree’ may be used after ‘list_concat’ [-Wuse-after-free]
550 | checkExprHasSubLink((Node *) newjointree);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../home/andres/src/postgresql/src/backend/rewrite/rewriteHandler.c:542:33: note: call to ‘list_concat’ here
542 | list_concat(newjointree, sub_action->jointree->fromlist);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Both of these bugs seem somewhat older, the latter going back to 19ff959bff0,
in 2005. I'm a bit surprised that we haven't hit them before, via
DEBUG_LIST_MEMORY_USAGE?

When compiling with optimization, another issue is reported:

[508/1814 22 28%] Compiling C object src/backend/postgres_lib.a.p/commands_explain.c.o
../../../../home/andres/src/postgresql/src/backend/commands/explain.c: In function 'ExplainNode':
../../../../home/andres/src/postgresql/src/backend/commands/explain.c:2037:25: warning: pointer 'ancestors' may be used after 'lcons' [-Wuse-after-free]
2037 | show_upper_qual(plan->qual, "Filter", planstate, ancestors, es);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'show_group_keys',
inlined from 'ExplainNode' at ../../../../home/andres/src/postgresql/src/backend/commands/explain.c:2036:4:
../../../../home/andres/src/postgresql/src/backend/commands/explain.c:2564:21: note: call to 'lcons' here
2564 | ancestors = lcons(plan, ancestors);
| ^~~~~~~~~~~~~~~~~~~~~~

which looks like it might be valid - the caller's "ancestors" variable could
now be freed? There do appear to be further instances of the issue, e.g. in
show_agg_keys(), that aren't flagged for some reason.

For something like pg_list.h the malloc(free) attribute is a bit awkward to
use, because one a) needs to list ~30 functions that can free a list and b)
the referenced functions need to be declared.

In my quick hack I just duplicated the relevant part of pg_list.h and added
the appropriate attributes to the copy of the original declaration.

I also added such attributes to bitmapset.h and palloc() et al, but that
didn't find existing bugs. It does find use-after-free instances if I add
some, similarly it does find cases of mismatching palloc with free etc.

The attached prototype is quite rough and will likely fail on anything but a
recent gcc (likely >= 12).

Do others think this would be useful enough to be worth polishing? And do you
agree the warnings above are bugs?

Greetings,

Andres Freund

Attachment Content-Type Size
v1-0001-Add-allocator-attributes.patch text/x-diff 15.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-06-26 20:00:23 Re: Analyze on table creation?
Previous Message Pavel Stehule 2023-06-26 19:38:03 Re: Analyze on table creation?