From 1b49910de755e11425cbe98e27eb7959c6774313 Mon Sep 17 00:00:00 2001 From: Oleg Tselebrovskiy Date: Tue, 14 Feb 2023 13:17:04 +0700 Subject: [PATCH 1/2] Fix some incorrect memory handling While running TimescaleDB under valgrind I've found two cases of incorrect memory handling. Case 1: When creating timescaledb extension, during the insertion of metadata there is some junk in memory that is not zeroed before writing there. Changes in metadata.c fix this. Case 2: When executing GRANT smth ON ALL TABLES IN SCHEMA some_schema and deconstructing this statement into granting to individual tables, process of copying names of those tables is wrong. Currently, you aren't copying the data itself, but an address to data on a page in some buffer. There's a problem - when the page in this buffer changes, copied address would lead to wrong data. Changes in process_utility.c fix this by allocating memory and then copying needed relname there. Fixes #5311 --- CHANGELOG.md | 5 +++++ src/process_utility.c | 3 ++- src/ts_catalog/metadata.c | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f11190c143..20e13c1517f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ Sooner to that time, we will announce the specific version of TimescaleDB in whi **Bugfixes** * #5214 Fix use of prepared statement in async module * #5218 Add role-level security to job error log +* #5290 Compression can't be enabled on continuous aggregates when segmentby/orderby columns need quotation +* #5311 Fix incorrect memory handling when working with names * #5239 Fix next_start calculation for fixed schedules * #5290 Fix enabling compression on continuous aggregates with columns requiring quotation @@ -49,6 +51,9 @@ Sooner to that time, we will announce the specific version of TimescaleDB in whi **Thanks** * @justinozavala for reporting an issue with PL/Python procedures in the background worker +**Thanks** +* @Medvecrab for reporting incorrect memory handling when working with names + ## 2.9.3 (2023-02-03) This release contains bug fixes since the 2.9.2 release and a fix for a security vulnerability (#5259). diff --git a/src/process_utility.c b/src/process_utility.c index 04d81a79f70..38531cc0cd3 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -1505,7 +1505,8 @@ process_relations_in_namespace(GrantStmt *stmt, Name schema_name, Oid namespaceI while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) { - Name relname = &((Form_pg_class) GETSTRUCT(tuple))->relname; + Name relname = palloc(NAMEDATALEN); + namestrcpy(relname, NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname)); /* these are being added for the first time into this list */ process_grant_add_by_name(stmt, false, schema_name, relname); diff --git a/src/ts_catalog/metadata.c b/src/ts_catalog/metadata.c index 7d36c656bda..71ea8d7e203 100644 --- a/src/ts_catalog/metadata.c +++ b/src/ts_catalog/metadata.c @@ -141,7 +141,7 @@ ts_metadata_insert(const char *metadata_key, Datum metadata_value, Oid type, bool isnull = false; Catalog *catalog = ts_catalog_get(); Relation rel; - char key_data[NAMEDATALEN]; + NameData key_data; rel = table_open(catalog_get_table_id(catalog, METADATA), ShareRowExclusiveLock); @@ -157,10 +157,10 @@ ts_metadata_insert(const char *metadata_key, Datum metadata_value, Oid type, /* We have to copy the key here because heap_form_tuple will copy NAMEDATALEN * into the tuple instead of checking length. */ - strlcpy(key_data, metadata_key, NAMEDATALEN); + namestrcpy(&key_data, metadata_key); /* Insert into the catalog table for persistence */ - values[AttrNumberGetAttrOffset(Anum_metadata_key)] = CStringGetDatum(key_data); + values[AttrNumberGetAttrOffset(Anum_metadata_key)] = NameGetDatum(&key_data); values[AttrNumberGetAttrOffset(Anum_metadata_value)] = convert_type_to_text(metadata_value, type); values[AttrNumberGetAttrOffset(Anum_metadata_include_in_telemetry)] = From c770a8631969bba6b05b9d3fb7a8d8555482a1f5 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Mon, 20 Feb 2023 12:25:50 +0100 Subject: [PATCH 2/2] Fix changelog message for NameData issue --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20e13c1517f..d1d3f730b59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,12 @@ accidentally triggering the load of a previous DB version.** ## Unreleased +**Bugfixes** * #5336 Use NameData and namestrcpy for names +* #5317 Fix some incorrect memory handling + +**Thanks** +* @Medvecrab for discovering an issue with copying NameData when forming heap tuples. ## 2.10.0 (2023-02-21) @@ -40,8 +45,6 @@ Sooner to that time, we will announce the specific version of TimescaleDB in whi **Bugfixes** * #5214 Fix use of prepared statement in async module * #5218 Add role-level security to job error log -* #5290 Compression can't be enabled on continuous aggregates when segmentby/orderby columns need quotation -* #5311 Fix incorrect memory handling when working with names * #5239 Fix next_start calculation for fixed schedules * #5290 Fix enabling compression on continuous aggregates with columns requiring quotation @@ -51,9 +54,6 @@ Sooner to that time, we will announce the specific version of TimescaleDB in whi **Thanks** * @justinozavala for reporting an issue with PL/Python procedures in the background worker -**Thanks** -* @Medvecrab for reporting incorrect memory handling when working with names - ## 2.9.3 (2023-02-03) This release contains bug fixes since the 2.9.2 release and a fix for a security vulnerability (#5259).