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 |
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? |