BUG #16837: Invalid memory access on \h in psql
The following bug has been logged on the website:
Bug reference: 16837
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 13.1
Operating system: Ubuntu 20.04
Description:
When executing in psql (under valgrind):
\h\
valgrind detects the following error:
==00:00:00:00.000 3226182==
==00:00:00:04.045 3226182== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:04.045 3226182== at 0x1396CB: helpSQL (help.c:600)
==00:00:00:04.045 3226182== by 0x120705: exec_command_help
(command.c:1507)
==00:00:00:04.045 3226182== by 0x1252CD: exec_command (command.c:351)
==00:00:00:04.045 3226182== by 0x1258A3: HandleSlashCmds
(command.c:222)
==00:00:00:04.045 3226182== by 0x13B166: MainLoop (mainloop.c:502)
==00:00:00:04.045 3226182== by 0x1238B3: process_file (command.c:3921)
==00:00:00:04.045 3226182== by 0x14357A: main (startup.c:400)
==00:00:00:04.045 3226182== Uninitialised value was created by a heap
allocation
==00:00:00:04.045 3226182== at 0x483B7F3: malloc (in
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==00:00:00:04.045 3226182== by 0x4AB2A1C: initPQExpBuffer
(pqexpbuffer.c:94)
==00:00:00:04.045 3226182== by 0x13D9CE: psql_scan_slash_option
(psqlscanslash.l:563)
==00:00:00:04.045 3226182== by 0x1206B6: exec_command_help
(command.c:1493)
==00:00:00:04.045 3226182== by 0x1252CD: exec_command (command.c:351)
==00:00:00:04.045 3226182== by 0x1258A3: HandleSlashCmds
(command.c:222)
==00:00:00:04.045 3226182== by 0x13B166: MainLoop (mainloop.c:502)
==00:00:00:04.045 3226182== by 0x1238B3: process_file (command.c:3921)
==00:00:00:04.045 3226182== by 0x14357A: main (startup.c:400)
==00:00:00:04.045 3226182==
{
<insert_a_suppression_name_here>
Memcheck:Cond
fun:helpSQL
fun:exec_command_help
fun:exec_command
fun:HandleSlashCmds
fun:MainLoop
fun:process_file
fun:main
}
No help available for "\".
Try \h with no arguments to see available help.
psql is started with the following command line:
valgrind --leak-check=no --track-origins=yes --time-stamp=yes
--read-var-info=yes \
--gen-suppressions=all --suppressions=src/tools/valgrind.supp \
--trace-children=yes $PGROOT/usr/local/pgsql/bin/psql
At Tue, 26 Jan 2021 07:00:00 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
When executing in psql (under valgrind):
\h\valgrind detects the following error:
==00:00:00:00.000 3226182==
==00:00:00:04.045 3226182== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:04.045 3226182== at 0x1396CB: helpSQL (help.c:600)
==00:00:00:04.045 3226182== by 0x120705: exec_command_help
(command.c:1507)
==00:00:00:04.045 3226182== by 0x1252CD: exec_command (command.c:351)
==00:00:00:04.045 3226182== by 0x1258A3: HandleSlashCmds
(command.c:222)
This is reproducible on master HEAD. helpSQL assumes that the first
word is longer than two characters and the second word exists. It also
doesn't care overruns. Addition to those issues, it miscounts the
length of the first two words if the third word exists.
=# \h ALTER VIEX HOGE
<prints help only of "ALTER VIEW"!, not of "ALTER *">
if (x > 1) /* Nothing on first pass - try the opening
* word(s) */
{
wordlen = j = 1;
!> while (topic[j] != ' ' && j++ < len)
wordlen++;
if (x == 2)
{
j++;
!> while (topic[j] != ' ' && j++ <= len)
wordlen++;
}
So we should check j before accessing topic[j] and count the length
correctly. The attached fixes that. This seems to be very old code.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
fix-psql-help-parse.patchtext/x-patch; charset=us-asciiDownload+4-4
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
At Tue, 26 Jan 2021 07:00:00 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
When executing in psql (under valgrind):
\h\
valgrind detects the following error:
==00:00:00:00.000 3226182==
==00:00:00:04.045 3226182== Conditional jump or move depends on
uninitialised value(s)
This is reproducible on master HEAD. helpSQL assumes that the first
word is longer than two characters and the second word exists. It also
doesn't care overruns. Addition to those issues, it miscounts the
length of the first two words if the third word exists.
Weirdly, valgrind isn't whining about this for me. But I agree that
that loop is unsafe. There are other problems too I think: neither
the initialization of "output" nor the calculation of nl_count seem
to be done sanely. This function really needs thoroughgoing review :-(
regards, tom lane
At Tue, 26 Jan 2021 11:11:22 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
At Tue, 26 Jan 2021 07:00:00 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
When executing in psql (under valgrind):
\h\
valgrind detects the following error:
==00:00:00:00.000 3226182==
==00:00:00:04.045 3226182== Conditional jump or move depends on
uninitialised value(s)This is reproducible on master HEAD. helpSQL assumes that the first
word is longer than two characters and the second word exists. It also
doesn't care overruns. Addition to those issues, it miscounts the
length of the first two words if the third word exists.Weirdly, valgrind isn't whining about this for me. But I agree that
that loop is unsafe. There are other problems too I think: neither
the initialization of "output" nor the calculation of nl_count seem
to be done sanely. This function really needs thoroughgoing review :-(
It looks far better now. Thanks!
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center