Thread: BUG #15540: Use after release in ExecuteTruncateGuts
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
=?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
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
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
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