pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16837: Invalid memory access on \h in psql
4+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2021-01-26 f76a85000b 82f97d33f4 3fa7b9078f 2c2e134b73 131825cd50 564cb2579a 64bdb6e5f8 Code review for psql's helpSQL() function.
* BUG #16837: Invalid memory access on \h in psql
@ 2021-01-26 07:00 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: PG Bug reporting form @ 2021-01-26 07:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #16837: Invalid memory access on \h in psql
@ 2021-01-26 11:17 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: Kyotaro Horiguchi @ 2021-01-26 11:17 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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.patch (text/x-patch, 625B) download
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 */
* Re: BUG #16837: Invalid memory access on \h in psql
@ 2021-01-26 16:11 Tom Lane <tgl@sss.pgh.pa.us>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2021-01-26 16:11 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #16837: Invalid memory access on \h in psql
@ 2021-01-27 01:34 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 4+ messages in thread
From: Kyotaro Horiguchi @ 2021-01-27 01:34 UTC (permalink / raw)
To: tgl@sss.pgh.pa.us; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2021-01-27 01:34 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 07:00 BUG #16837: Invalid memory access on \h in psql PG Bug reporting form <noreply@postgresql.org>
2021-01-26 11:17 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2021-01-26 16:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-01-27 01:34 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox