Date: Wed, 13 Nov 2024 20:52:11 +0530
From: Jayesh Dehankar <jayesh.dp@zohocorp.com>
To: "pgsql-hackers" <pgsql-hackers@postgresql.org>
Cc: "zlabs-cstore@zohocorp.com" <zlabs-cstore@zohocorp.com>,
	"pgsql-hackers" <pgsql-hackers@lists.postgresql.org>,
	"pgsql-bugs" <pgsql-bugs@lists.postgresql.org>
Message-Id: <193261e2c4d.3dd3cd7c1842.871636075166132237@zohocorp.com>
In-Reply-To: 
Subject: [Bug] Heap Use After Free in Window Aggregate Execution
MIME-Version: 1.0
Content-Type: multipart/mixed;
	boundary="----=_Part_6447_1211805977.1731511331918"
Importance: Medium
User-Agent: Zoho Mail
Archived-At: 
 <https://www.postgresql.org/message-id/193261e2c4d.3dd3cd7c1842.871636075166132237%40zohocorp.com>
Precedence: bulk

------=_Part_6447_1211805977.1731511331918
Content-Type: multipart/alternative;
	boundary="----=_Part_6448_1577446894.1731511331918"

------=_Part_6448_1577446894.1731511331918
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Developers,


We have discovered a bug in PostgreSQL v16.3 related to a top-level window =
aggregate with a partition-by clause. The issue occurs when the run conditi=
on fails, causing the window aggregate status to change from WINDOWAGG_RUN =
to WINDOWAGG_PASSTHROUGH_STRICT mode. The bug is present in the latest STAB=
LE branch.


What's the Issue?


During window function execution, the first window function is evaluated, a=
nd the per-tuple result is stored in econtext->ecxt_aggvalues. For instance=
, if a window function like lead is evaluated on varlena columns, the resul=
t is copied to (econtext)->ecxt_per_tuple_memory. This memory is reset befo=
re beginning window function evaluation for each row. When the run conditio=
n fails for a row in pass-through mode, the window aggregate status changes=
 to WINDOWAGG_PASSTHROUGH_STRICT, and execution proceeds to the next tuple =
if rows remain in the partition.


During the next tuple's execution, (econtext)->ecxt_per_tuple_memory is res=
et, and the window function is skipped due to WINDOWAGG_PASSTHROUGH_STRICT =
mode. However, the ExecProject function still executes for the tuple to sto=
re aggregate results in the result slot. Since econtext->ecxt_aggvalues is =
not set to NULL after the run condition fails, it attempts to access the pr=
evious tuple's window function result for the current row. When the window =
function operates on varlena columns, this results in invalid memory access=
 at MakeExpandedObjectReadOnlyInternal(state->resvalue); (execExprInterp.c)=
.


This bug occurs immediately after a failed run condition, particularly when=
 using window functions like lead() on varlena columns.


Question: Why does ExecProject still execute if the current window aggregat=
e status is WINDOWAGG_PASSTHROUGH_STRICT mode?


Possible Fixes


1) Skip the partition immediately after a run condition fails for a row, us=
ing windowaggstatus and winstate->partition_spooled info to begin processin=
g the next partition (windowagg_fix1.patch)


2) Set aggregate values to NULL upon run condition failure in WINDOWAGG_PAS=
STHROUGH_STRICT mode (windowagg_fix2.patch)


How was the Bug Detected?


In our private project, we use a custom malloc allocator to enhance Address=
Sanitizer's effectiveness in detecting memory-related bugs. Up to PostgreSQ=
L v14, this allocator was implemented as a custom solution. However, starti=
ng from version 16, PostgreSQL restricts the use of custom allocators. Cons=
equently, we integrated this allocator directly into PostgreSQL and are usi=
ng it to run regressions (malloc_allocator.patch)


Our custom allocator precisely allocates the user's requested size, and pfr=
ee operations immediately release pointers without adding them to any freel=
ist. Though this approach incurs performance degradation, it allows Address=
Sanitizer to detect use-after-free bugs more effectively.


Why Aset Context Missed the Bug:

The Aset context did not detect the bug because it did not immediately free=
 pointers, instead added them to a freelist.


Steps to Reproduce the Issue Using Custom Malloc Allocator


1) Apply the attached malloc_allocator.patch to the master branch (#commit:=
 bfeeb065ea2c870cf4d9dfcd552d23d72432e692) and compile PG with the --enable=
-asan=C2=A0flag.


2) Execute below SQL queries:

create table issue(a int, f text);

insert into issue values (1, 'aa'), (1, 'bb');

select * from ( select row_number() over (partition by a) as first, lead(f)=
 over (partition by a) as third from issue) emp where first < 1;


NOTE: Running regressions with our custom allocator may reveal additional m=
emory-related bugs or crashes in the master branch.


Please let me know if you have any questions or would like further details.


Thanks & Regards,
Jayesh Dehankar
Member Technical Staff
ZOHO Corporation
------=_Part_6448_1577446894.1731511331918
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"><html><head>=
<meta content=3D"text/html;charset=3DUTF-8" http-equiv=3D"Content-Type"></h=
ead><body ><div style=3D"font-family: Verdana, Arial, Helvetica, sans-serif=
; font-size: 10pt;"><div>Hi Developers,<br></div><div><br></div><div>We hav=
e discovered a bug in PostgreSQL v16.3 related to a top-level window aggreg=
ate with a partition-by clause. The issue occurs when the run condition fai=
ls, causing the window aggregate status to change from WINDOWAGG_RUN to WIN=
DOWAGG_PASSTHROUGH_STRICT mode. The bug is present in the latest STABLE bra=
nch.<br></div><div><br></div><div><br></div><div><b>What's the Issue?</b><b=
r></div><div><br></div><div>During window function execution, the first win=
dow function is evaluated, and the per-tuple result is stored in econtext-&=
gt;ecxt_aggvalues. For instance, if a window function like lead is evaluate=
d on varlena columns, the result is copied to (econtext)-&gt;ecxt_per_tuple=
_memory. This memory is reset before beginning window function evaluation f=
or each row. When the run condition fails for a row in pass-through mode, t=
he window aggregate status changes to WINDOWAGG_PASSTHROUGH_STRICT, and exe=
cution proceeds to the next tuple if rows remain in the partition.<br></div=
><div><br></div><div>During the next tuple's execution, (econtext)-&gt;ecxt=
_per_tuple_memory is reset, and the window function is skipped due to WINDO=
WAGG_PASSTHROUGH_STRICT mode. However, the ExecProject function still execu=
tes for the tuple to store aggregate results in the result slot. Since econ=
text-&gt;ecxt_aggvalues is not set to NULL after the run condition fails, i=
t attempts to access the previous tuple's window function result for the cu=
rrent row. When the window function operates on varlena columns, this resul=
ts in invalid memory access at MakeExpandedObjectReadOnlyInternal(state-&gt=
;resvalue); (execExprInterp.c).<br></div><div><br></div><div>This bug occur=
s immediately after a failed run condition, particularly when using window =
functions like lead() on varlena columns.<br></div><div><br></div><div><i>Q=
uestion:</i> Why does ExecProject still execute if the current window aggre=
gate status is WINDOWAGG_PASSTHROUGH_STRICT mode?<br></div><div><br></div><=
div><br></div><div><b>Possible Fixes</b><br></div><div><br></div><div>1) Sk=
ip the partition immediately after a run condition fails for a row, using w=
indowaggstatus and winstate-&gt;partition_spooled info to begin processing =
the next partition (windowagg_fix1.patch)<br><br></div><div>2) Set aggregat=
e values to NULL upon run condition failure in WINDOWAGG_PASSTHROUGH_STRICT=
 mode (windowagg_fix2.patch)<br></div><div><br></div><div><br></div><div><b=
>How was the Bug Detected?</b><br></div><div><br></div><div>In our private =
project, we use a custom malloc allocator to enhance AddressSanitizer's eff=
ectiveness in detecting memory-related bugs. Up to PostgreSQL v14, this all=
ocator was implemented as a custom solution. However, starting from version=
 16, PostgreSQL restricts the use of custom allocators. Consequently, we in=
tegrated this allocator directly into PostgreSQL and are using it to run re=
gressions (malloc_allocator.patch)<br></div><div><br></div><div>Our custom =
allocator precisely allocates the user's requested size, and pfree operatio=
ns immediately release pointers without adding them to any freelist. Though=
 this approach incurs performance degradation, it allows AddressSanitizer t=
o detect use-after-free bugs more effectively.<br></div><div><br></div><div=
><i>Why Aset Context Missed the Bug</i>:<br></div><div>The Aset context did=
 not detect the bug because it did not immediately free pointers, instead a=
dded them to a freelist.<br></div><div><br></div><div><br></div><div><b>Ste=
ps to Reproduce the Issue Using Custom Malloc Allocator</b><br></div><div><=
br></div><div>1) Apply the attached malloc_allocator.patch to the master br=
anch (#commit: bfeeb065ea2c870cf4d9dfcd552d23d72432e692) and compile PG wit=
h the <i>--enable-asan</i>&nbsp;flag.<br></div><div><br></div><div><div>2) =
Execute below SQL queries:<br></div><table cellpadding=3D"2" cellspacing=3D=
"2" border=3D"1" style=3D"font-size: 10pt; font-family: Verdana, Arial, Hel=
vetica, sans-serif; width: 100%; border-collapse: collapse; border: 1px sol=
id rgb(171, 171, 171);"><tbody><tr><td style=3D"border: 1px solid rgb(171, =
171, 171); vertical-align: top; width: 1059px;"><div><div>create table issu=
e(a int, f text);<br></div><div>insert into issue values (1, 'aa'), (1, 'bb=
');<br></div><div>select * from ( select row_number() over (partition by a)=
 as first, lead(f) over (partition by a) as third from issue) emp where fir=
st &lt; 1;<br></div></div></td></tr></tbody></table><div><br></div><div><b>=
<i>NOTE:</i></b> Running regressions with our custom allocator may reveal a=
dditional memory-related bugs or crashes in the master branch.<br></div><di=
v><br></div><div>Please let me know if you have any questions or would like=
 further details.<br></div><div><br></div><div><br>Thanks &amp; Regards,<br=
>Jayesh Dehankar<br>Member Technical Staff<br>ZOHO Corporation</div></div><=
/div><br></body></html>
------=_Part_6448_1577446894.1731511331918--

------=_Part_6447_1211805977.1731511331918
Content-Type: application/octet-stream; name=windowagg_fix1.patch
Content-Transfer-Encoding: 7bit
X-ZM_AttachId: 139508041319180110
Content-Disposition: attachment; filename=windowagg_fix1.patch