Content-Type: multipart/mixed; boundary="------------PO2ZIHf5SmcK38grIqOydDIJ"
Message-ID: <591971ce-25c1-90f3-0526-5f54e3ebb32e@gmail.com>
Date: Wed, 7 Jun 2023 11:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: Importing pg_bsd_indent into our source tree
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org
References: <20230209215532.oomwhj5jyggmciq7@awork3.anarazel.de>
 <20230209221450.bkuozhhzlgpupx4d@awork3.anarazel.de>
 <3966340.1675981162@sss.pgh.pa.us>
 <20230209231013.gjpxs4gapxjatc64@awork3.anarazel.de>
 <3972584.1675984746@sss.pgh.pa.us>
 <20230210000841.4chvtmr4gkrfsotb@awork3.anarazel.de>
 <3979957.1675988437@sss.pgh.pa.us>
 <20230210003555.kyhyqq3imgocs62k@awork3.anarazel.de>
 <3986263.1675989733@sss.pgh.pa.us> <718245.1676159640@sss.pgh.pa.us>
 <20230212004254.3lp22a7bpkcjo3y6@awork3.anarazel.de>
 <734490.1676165550@sss.pgh.pa.us> <736439.1676166232@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <736439.1676166232@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/591971ce-25c1-90f3-0526-5f54e3ebb32e%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------PO2ZIHf5SmcK38grIqOydDIJ
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hello,

I've discovered that the pg_bsd_indent test added here makes an
ASAN-instrumented build fail on:
ASAN_OPTIONS=detect_leaks=0:strict_string_checks=1 make check-world
as follows:
# +++ tap check in src/tools/pg_bsd_indent +++
t/001_pg_bsd_indent.pl .. 1/?
#   Failed test 'pg_bsd_indent succeeds on binary'
#   at t/001_pg_bsd_indent.pl line 41.

#   Failed test 'pg_bsd_indent output matches for binary'
#   at t/001_pg_bsd_indent.pl line 50.

#   Failed test 'pg_bsd_indent succeeds on comments'
#   at t/001_pg_bsd_indent.pl line 41.

#   Failed test 'pg_bsd_indent output matches for comments'
#   at t/001_pg_bsd_indent.pl line 50.
t/001_pg_bsd_indent.pl .. 10/?
...
regress_log_001_pg_bsd_indent contains:
# Running: pg_bsd_indent .../src/tools/pg_bsd_indent/tests/binary.0 binary.out 
-P.../src/tools/pg_bsd_indent/tests/binary.pro
=================================================================
==2124067==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60600000005a at pc 0x560aa8972dae bp 
0x7ffebd080460 sp 0x7ffebd07fc08
READ of size 40 at 0x60600000005a thread T0
     #0 0x560aa8972dad in __interceptor_strspn.part.0 (.../src/tools/pg_bsd_indent/pg_bsd_indent+0x5edad)
     #1 0x560aa8a3f495 in lexi .../src/tools/pg_bsd_indent/lexi.c:258
     #2 0x560aa8a3451d in main .../src/tools/pg_bsd_indent/indent.c:269
     #3 0x7f6ffab1ad8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
     #4 0x7f6ffab1ae3f in __libc_start_main_impl ../csu/libc-start.c:392
     #5 0x560aa894b7d4 in _start (.../src/tools/pg_bsd_indent/pg_bsd_indent+0x377d4)

0x60600000005a is located 0 bytes to the right of 58-byte region [0x606000000020,0x60600000005a)
allocated by thread T0 here:
     #0 0x560aa89e4980 in __interceptor_realloc.part.0 (.../src/tools/pg_bsd_indent/pg_bsd_indent+0xd0980)
     #1 0x560aa8a3d24e in fill_buffer .../src/tools/pg_bsd_indent/io.c:365

SUMMARY: AddressSanitizer: heap-buffer-overflow (.../src/tools/pg_bsd_indent/pg_bsd_indent+0x5edad) in 
__interceptor_strspn.part.0
...

I understand that that code is almost as ancient as me and it works as
intended (fill_buffer() doesn't null-terminate a buffer), but may be this is
worth fixing in the Postgres source tree to keep the whole testing baseline
high (if someone finds strict_string_checks useful). If so, perhaps
something like the attached will do.

Best regards,
Alexander
--------------PO2ZIHf5SmcK38grIqOydDIJ
Content-Type: text/x-patch; charset=UTF-8;
 name="improve-strspn-for-bsd_indent.patch"
Content-Disposition: attachment;
 filename="improve-strspn-for-bsd_indent.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy90b29scy9wZ19ic2RfaW5kZW50L2xleGkuYyBiL3NyYy90b29s
cy9wZ19ic2RfaW5kZW50L2xleGkuYwppbmRleCA5NDNiZjdjZTZiLi4xYWY4MTY3NDkzIDEw
MDY0NAotLS0gYS9zcmMvdG9vbHMvcGdfYnNkX2luZGVudC9sZXhpLmMKKysrIGIvc3JjL3Rv
b2xzL3BnX2JzZF9pbmRlbnQvbGV4aS5jCkBAIC0yMTIsNiArMjEyLDI2IEBAIGlzX2Z1bmNf
ZGVmaW5pdGlvbihjaGFyICp0cCkKICAgICByZXR1cm4gZmFsc2U7CiB9CiAKK3N0YXRpYyBz
aXplX3QKK3N0cnNwbl9uKGNvbnN0IGNoYXIgKnMxLCBjb25zdCBjaGFyICpzMikKK3sKKwlj
b25zdCBjaGFyICpwID0gczEsICpzcGFucDsKKwljaGFyIGMsIHNjOworCS8qCisJICogU2tp
cCBhbnkgY2hhcmFjdGVycyBpbiBzMiwgZXhjbHVkaW5nIHRoZSB0ZXJtaW5hdGluZyBcMCBh
bmQgXG4uCisJICovCitjb250OgorCWMgPSAqcCsrOworCWZvciAoc3BhbnAgPSBzMjsgKHNj
ID0gKnNwYW5wKyspICE9IDA7KQorCQlpZiAoc2MgPT0gYykKKwkJCWdvdG8gY29udDsKKwkJ
ZWxzZSBpZiAoc2MgPT0gJ1xuJykKKwkJCWJyZWFrOworCXJldHVybiAocCAtIDEgLSBzMSk7
Cit9CisKKyNkZWZpbmUgc3Ryc3BuKHMxLCBzMikgc3Ryc3BuX24oczEsIHMyKQorCiBpbnQK
IGxleGkoc3RydWN0IHBhcnNlcl9zdGF0ZSAqc3RhdGUpCiB7Cg==

--------------PO2ZIHf5SmcK38grIqOydDIJ--