| From: | Feilong Meng <feelingmeng(at)foxmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
| Subject: | Re: Fix uninitialized PruneFreezeResult in pruneheap and vacuumlazy |
| Date: | 2025-12-11 10:17:39 |
| Message-ID: | tencent_F99BAF3F98799941EF31EEEE23E34A0B2F05@qq.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Chao Li,Hackers,
> PruneFreezeResult presult; <== here presult is not initialized
> const PruneFreezeResult *presult, <== presult is a const pointer, so prune_freeze_setup won’t update its content
> PruneState *prstate)
> {
> prstate->deadoffsets = (OffsetNumber *) presult->deadoffsets; <== presult->deadoffsets could be a random value
> }
> ```
> Attached is a simple fix by just initializing presult in the first place with {0}.
Initializing ’presult‘ under my operating system is very effective.
I have tested the change, and "make check" passed.
Best regards!
Feilong Meng
feelingmeng(at)foxmail(dot)com
HighGo Software Co., Ltd.
Original
From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Date: 2025-12-11 12:02
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Melanie Plageman <melanieplageman(at)gmail(dot)com>
Subject: Fix uninitialized PruneFreezeResult in pruneheap and vacuumlazy
Hi Hackers,
While reviewing Melanie's patch [1], I found this bug where presult is not initialized. Let me explain the logic.
In the first place:
```
static int
lazy_scan_prune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
Page page,
Buffer vmbuffer,
bool all_visible_according_to_vm,
bool *has_lpdead_items,
bool *vm_page_frozen)
{
Relation rel = vacrel->rel;
PruneFreezeResult presult; <== here presult is not initialized
heap_page_prune_and_freeze(&params,
&presult, <== uninitialized presult is passed into heap_page_prune_and_freeze
&vacrel->offnum,
&vacrel->NewRelfrozenXid, &vacrel->NewRelminMxid);
```
Then in heap_page_prune_and_freeze():
```
void
heap_page_prune_and_freeze(PruneFreezeParams *params,
PruneFreezeResult *presult,
OffsetNumber *off_loc,
TransactionId *new_relfrozen_xid,
MultiXactId *new_relmin_mxid)
{
Buffer buffer = params->buffer;
Page page = BufferGetPage(buffer);
PruneState prstate;
bool do_freeze;
bool do_prune;
bool do_hint_prune;
bool did_tuple_hint_fpi;
int64 fpi_before = pgWalUsage.wal_fpi;
/* Initialize prstate */
prune_freeze_setup(params,
new_relfrozen_xid, new_relmin_mxid,
presult, &prstate); <== immediately pass uninitialized presult to prune_freeze_setup
```
Then in prune_freeze_setup():
```
static void
prune_freeze_setup(PruneFreezeParams *params,
TransactionId *new_relfrozen_xid,
MultiXactId *new_relmin_mxid,
const PruneFreezeResult *presult, <== presult is a const pointer, so prune_freeze_setup won’t update its content
PruneState *prstate)
{
prstate->deadoffsets = (OffsetNumber *) presult->deadoffsets; <== presult->deadoffsets could be a random value
}
```
Attached is a simple fix by just initializing presult in the first place with {0}.
[1] https://postgr.es/m/CAAKRu_bvQiesumRhgvbcym1T9Z9=ddGfUbi-dSNxLRc6JvL1-w@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ajit Awekar | 2025-12-11 10:52:33 | Re: Periodic authorization expiration checks using GoAway message |
| Previous Message | Chao Li | 2025-12-11 10:00:20 | Docs: Standardize "cannot" usage in SGML source |