Обсуждение: 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
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4883ebd2ed..54fe099ad7 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -592,12 +592,12 @@ helpSQL(const char *topic, unsigned short int pager)
* word(s) */
{
wordlen = j = 1;
- while (topic[j] != ' ' && j++ < len)
+ while (j < len && topic[j++] != ' ')
wordlen++;
- if (x == 2)
+ if (x == 2 && j < len)
{
- j++;
- while (topic[j] != ' ' && j++ <= len)
+ wordlen++;
+ while (j < len && topic[j++] != ' ')
wordlen++;
}
if (wordlen >= len) /* Don't try again if the same word */
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