Обсуждение: 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