Skip to content

Commit

Permalink
Fix some incorrect memory handling
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Medvecrab authored and mkindahl committed Feb 20, 2023
1 parent 7e43c70 commit 0746517
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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).
Expand Down
3 changes: 2 additions & 1 deletion src/process_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/ts_catalog/metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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)] =
Expand Down

0 comments on commit 0746517

Please sign in to comment.