pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedPrevent invalid memory access in LookupFuncName
2+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2019-06-25 52cdbdc42c 131e545ac3 Fix thinkos in LookupFuncName() for function name lookups
* Prevent invalid memory access in LookupFuncName
@ 2019-06-24 20:10 Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 2+ messages in thread
From: Alexander Lakhin @ 2019-06-24 20:10 UTC (permalink / raw)
To: pgsql-hackers
Hello hackers,
When running on REL_11_STABLE the following query:
CREATE PROCEDURE test_ambiguous_procname(int) as $$ begin end; $$
language plpgsql;
CREATE PROCEDURE test_ambiguous_procname(text) as $$ begin end; $$
language plpgsql;
DROP PROCEDURE test_ambiguous_procname;
under valgrind I get the memory access errors:
2019-06-24 22:21:39.925 MSK|law|regression|5d1122c2.2921|LOG:
statement: DROP PROCEDURE test_ambiguous_procname;
==00:00:00:07.756 10529== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:07.756 10529== at 0x4C35E60: __memcmp_sse4_1 (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==00:00:00:07.756 10529== by 0x2E9A7B: LookupFuncName (parse_func.c:2078)
==00:00:00:07.756 10529== by 0x2E9D96: LookupFuncWithArgs
(parse_func.c:2141)
==00:00:00:07.756 10529== by 0x2A2F7E: get_object_address
(objectaddress.c:893)
==00:00:00:07.756 10529== by 0x31C9C0: RemoveObjects (dropcmds.c:71)
==00:00:00:07.756 10529== by 0x50D4FF: ExecDropStmt (utility.c:1738)
==00:00:00:07.756 10529== by 0x5100BC: ProcessUtilitySlow
(utility.c:1580)
==00:00:00:07.756 10529== by 0x50EDFE: standard_ProcessUtility
(utility.c:835)
==00:00:00:07.756 10529== by 0x50F07D: ProcessUtility (utility.c:360)
==00:00:00:07.756 10529== by 0x50B4D2: PortalRunUtility (pquery.c:1178)
==00:00:00:07.756 10529== by 0x50C169: PortalRunMulti (pquery.c:1324)
==00:00:00:07.756 10529== by 0x50CEFF: PortalRun (pquery.c:799)
==00:00:00:07.756 10529== Uninitialised value was created by a stack
allocation
==00:00:00:07.756 10529== at 0x2E9C31: LookupFuncWithArgs
(parse_func.c:2106)
==00:00:00:07.756 10529==
...
==00:00:00:07.756 10529== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:07.756 10529== at 0x2E9A7E: LookupFuncName (parse_func.c:2078)
==00:00:00:07.756 10529== by 0x2E9D96: LookupFuncWithArgs
(parse_func.c:2141)
==00:00:00:07.757 10529== by 0x2A2F7E: get_object_address
(objectaddress.c:893)
==00:00:00:07.757 10529== by 0x31C9C0: RemoveObjects (dropcmds.c:71)
==00:00:00:07.757 10529== by 0x50D4FF: ExecDropStmt (utility.c:1738)
==00:00:00:07.757 10529== by 0x5100BC: ProcessUtilitySlow
(utility.c:1580)
==00:00:00:07.757 10529== by 0x50EDFE: standard_ProcessUtility
(utility.c:835)
==00:00:00:07.757 10529== by 0x50F07D: ProcessUtility (utility.c:360)
==00:00:00:07.757 10529== by 0x50B4D2: PortalRunUtility (pquery.c:1178)
==00:00:00:07.757 10529== by 0x50C169: PortalRunMulti (pquery.c:1324)
==00:00:00:07.757 10529== by 0x50CEFF: PortalRun (pquery.c:799)
==00:00:00:07.757 10529== by 0x5090FF: exec_simple_query
(postgres.c:1145)
==00:00:00:07.757 10529== Uninitialised value was created by a stack
allocation
==00:00:00:07.757 10529== at 0x2E9C31: LookupFuncWithArgs
(parse_func.c:2106)
As I see, the code in LookupFuncName can fall through the "if (nargs ==
-1)" condition and execute memcmp with nargs==-1.
The proposed patch is attached.
Best regards,
Alexander
Attachments:
fix-LookupFuncName.patch (text/x-patch, 437B) download
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 44257154b81..ee9f73ef977 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2071,6 +2071,7 @@ LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool noError)
errmsg("could not find a function named \"%s\"",
NameListToString(funcname))));
}
+ return InvalidOid;
}
while (clist)
* Re: Prevent invalid memory access in LookupFuncName
@ 2019-06-25 02:19 Michael Paquier <michael@paquier.xyz>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 0 replies; 2+ messages in thread
From: Michael Paquier @ 2019-06-25 02:19 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: pgsql-hackers
On Mon, Jun 24, 2019 at 11:10:54PM +0300, Alexander Lakhin wrote:
> When running on REL_11_STABLE the following query:
> CREATE PROCEDURE test_ambiguous_procname(int) as $$ begin end; $$
> language plpgsql;
> CREATE PROCEDURE test_ambiguous_procname(text) as $$ begin end; $$
> language plpgsql;
> DROP PROCEDURE test_ambiguous_procname;
> under valgrind I get the memory access errors:
Thanks! I have been able to reproduce the problem, and the error is
obvious looking at the code. I have changed the patch to be more
consistent with HEAD though, returning InvalidOid in the code paths
generating the error. The logic is the same, but that looked cleaner
to me, and I have added some comments on the way, similarly to what
bfb456c1 has done for HEAD (where LookupFuncNameInternal is doing the
right thing already). This has been incorrect since aefeb68, so
back-patched down to 10.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2019-06-25 02:19 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 20:10 Prevent invalid memory access in LookupFuncName Alexander Lakhin <exclusion@gmail.com>
2019-06-25 02:19 ` Michael Paquier <michael@paquier.xyz>
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