pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #15540: Use after release in ExecuteTruncateGuts
5+ messages / 5 participants
[nested] [flat]
* BUG #15540: Use after release in ExecuteTruncateGuts
@ 2018-12-07 10:34 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 5+ messages in thread
From: PG Bug reporting form @ 2018-12-07 10:34 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: bianpan2016@163.com
The following bug has been logged on the website:
Bug reference: 15540
Logged by: Pan Bian
Email address: bianpan2016@163.com
PostgreSQL version: 11.1
Operating system: Linux
Description:
File: src/backend/commands/tablecmds.c
Function: ExecuteTruncateGuts
Issue details:
The function ExecuteTruncateGuts drops the reference to rel via
relation_close when toast_relid is valid. However, after that, rel is passed
to pgstat_count_truncate. This may result in a use-after-release bug. Maybe,
rel should be re-declared on the branch that toast_relid is valid.
For your convenience, I copy-and-paste related code as follows:
void
ExecuteTruncateGuts(List *explicit_rels, List *relids, List
*relids_logged,
DropBehavior behavior, bool restart_seqs)
{
...
foreach(cell, rels)
{
Relation rel = (Relation) lfirst(cell);
...
if (rel->rd_createSubid == mySubid ||
rel->rd_newRelfilenodeSubid == mySubid)
{
/* Immediate, non-rollbackable truncation is OK */
heap_truncate_one_rel(rel);
}
else
{
Oid heap_relid;
Oid toast_relid;
...
toast_relid = rel->rd_rel->reltoastrelid;
/*
* The same for the toast table, if any.
*/
if (OidIsValid(toast_relid))
{
rel = relation_open(toast_relid, AccessExclusiveLock);
//### open a relation
RelationSetNewRelfilenode(rel,
rel->rd_rel->relpersistence,
RecentXmin, minmulti);
if (rel->rd_rel->relpersistence ==
RELPERSISTENCE_UNLOGGED)
heap_create_init_fork(rel);
heap_close(rel, NoLock); //### release the relation
}
/*
* Reconstruct the indexes to match, and we're done.
*/
reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, 0);
}
pgstat_count_truncate(rel); //### the released relation is used
again
}
}
Thank you,
Pan Bian
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #15540: Use after release in ExecuteTruncateGuts
@ 2018-12-07 16:09 Tom Lane <tgl@sss.pgh.pa.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 2 replies; 5+ messages in thread
From: Tom Lane @ 2018-12-07 16:09 UTC (permalink / raw)
To: bianpan2016@163.com; +Cc: pgsql-bugs@lists.postgresql.org
=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> The function ExecuteTruncateGuts drops the reference to rel via
> relation_close when toast_relid is valid. However, after that, rel is passed
> to pgstat_count_truncate. This may result in a use-after-release bug.
... and, even more to the point, the truncation stats count is incorrectly
applied to the toast table not its parent.
> Maybe,
> rel should be re-declared on the branch that toast_relid is valid.
Yeah, seems like the right way. Will fix.
Are you using a static analyzer to find these? I'm curious how
you noticed them.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #15540: Use after release in ExecuteTruncateGuts
@ 2018-12-07 17:22 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 5+ messages in thread
From: Alvaro Herrera @ 2018-12-07 17:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: bianpan2016@163.com, pgsql-bugs@lists.postgresql.org, Oleksandr Shulgin <oleksandr.shulgin@zalando.de>
On 2018-Dec-07, Tom Lane wrote:
> =?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> > The function ExecuteTruncateGuts drops the reference to rel via
> > relation_close when toast_relid is valid. However, after that, rel is passed
> > to pgstat_count_truncate. This may result in a use-after-release bug.
>
> ... and, even more to the point, the truncation stats count is incorrectly
> applied to the toast table not its parent.
Whoops! Thanks for fixing. You're right, we quite likely didn't
specifically test the case of there being a toast table. Amazing that
it took so long to find it.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #15540: Use after release in ExecuteTruncateGuts
@ 2018-12-09 00:56 PanBian <bianpan2016@163.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 5+ messages in thread
From: PanBian @ 2018-12-09 00:56 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
On Fri, Dec 07, 2018 at 11:09:05AM -0500, Tom Lane wrote:
> =?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> > The function ExecuteTruncateGuts drops the reference to rel via
> > relation_close when toast_relid is valid. However, after that, rel is passed
> > to pgstat_count_truncate. This may result in a use-after-release bug.
>
> ... and, even more to the point, the truncation stats count is incorrectly
> applied to the toast table not its parent.
>
> > Maybe,
> > rel should be re-declared on the branch that toast_relid is valid.
>
> Yeah, seems like the right way. Will fix.
>
> Are you using a static analyzer to find these? I'm curious how
> you noticed them.
Yes. I write a static analysis tool. It can find functions that release
memory or other resources. Let's call them free-like functions. With such
free-like functions, the tool then performs data flow analysis to find
use-after-free bugs. Of course, we can feed those free-like functions to
other static analyzers such as Coverity. I believe it will work too.
Best regards,
Pan Bian
>
> regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #15540: Use after release in ExecuteTruncateGuts
@ 2018-12-09 02:20 Michael Paquier <michael@paquier.xyz>
parent: PanBian <bianpan2016@163.com>
0 siblings, 0 replies; 5+ messages in thread
From: Michael Paquier @ 2018-12-09 02:20 UTC (permalink / raw)
To: PanBian <bianpan2016@163.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org
On Sun, Dec 09, 2018 at 08:56:17AM +0800, PanBian wrote:
> Yes. I write a static analysis tool. It can find functions that release
> memory or other resources. Let's call them free-like functions. With such
> free-like functions, the tool then performs data flow analysis to find
> use-after-free bugs. Of course, we can feed those free-like functions to
> other static analyzers such as Coverity. I believe it will work too.
Interesting. Did you release this stuff in the open? I could be very
interesting to get that plugged in more easily with Postgres. Community
runs Coverity as well. The reports are not public still if that helps
in reporting real issues and not only false positives that would be
nice.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2018-12-09 02:20 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07 10:34 BUG #15540: Use after release in ExecuteTruncateGuts PG Bug reporting form <noreply@postgresql.org>
2018-12-07 16:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-12-07 17:22 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2018-12-09 00:56 ` PanBian <bianpan2016@163.com>
2018-12-09 02:20 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox