NotHimmel opened a new pull request, #2521:
URL: https://github.com/apache/age/pull/2521
Fixes #2519
### Problem
`node_label` is declared `name = NULL` in the SQL signature of
`ag_catalog.age_create_barbell_graph()`, so leaving it out — or passing
`NULL`
explicitly — is a supported call. Both crashed the backend with SIGSEGV,
which
makes the postmaster reinitialize and drops every other session on the
instance.
```sql
SELECT create_graph('barbell_test');
SELECT age_create_barbell_graph('barbell_test', 5, 0);
-- server closed the connection unexpectedly
```
```
LOG: client backend (PID 312566) was terminated by signal 11: Segmentation
fault
LOG: all server processes terminated; reinitializing
```
### Root cause
There are two separate faults on the null-`node_label` path.
**1. The default label was copied into a null pointer.**
```c
Name node_label_name = NULL;
...
if (PG_ARGISNULL(3))
namestrcpy(node_label_name, AG_DEFAULT_LABEL_VERTEX);
```
`namestrcpy()` writes through the pointer it is given and does not allocate,
so
this writes to address 0. The fix gives the default its own `NameData`. As a
side effect the default now actually takes effect, which it never did.
Note that the immediate segfault only happens on PostgreSQL 14 and later.
`namestrcpy()` used to start with `if (!name || !str) return -1;`, removed by
PostgreSQL commit `1784f278a638` ("Replace remaining StrNCpy() by strlcpy()",
first released in 14). On PG 13 and earlier the call silently did nothing, so
`node_label_str` simply ended up NULL and the failure moved downstream.
**2. The node label was forwarded as the raw argument Datum.**
```c
DirectFunctionCall4(create_complete_graph, ..., arguments->args[3].value);
```
`DirectFunctionCall4()` marks every argument as not null, so a null node
label
reached `create_complete_graph()` as a non-null NULL pointer and was
dereferenced by its vertex/edge label comparison. Fixing only the
`namestrcpy()`
call is therefore not enough — verified: with just that change,
`age_create_barbell_graph('g', 5, 0, NULL, NULL, 'E')` still segfaults. The
resolved label is forwarded instead.
`create_complete_graph()` already handles its own null node label correctly;
this change makes the barbell function follow the same approach.
### How the bug was introduced
By the original barbell implementation, `0c79370` ("Barbell graph
generation",
#648, 2023-02-17). Present on `master`, `PG16`, `PG17`, `PG18` and `PG19`, so
the fix likely wants backporting to those branches.
### Testing
Extended `regress/sql/graph_generation.sql` with the previously untested
cases.
The existing barbell tests always passed a node label, except for the
all-arguments-null case, which errors out on the graph name before ever
reaching this code — which is why this was never caught.
```sql
SELECT * FROM age_create_barbell_graph('gp7',5,0,NULL,NULL,'edges',NULL);
SELECT COUNT(*) FROM gp7."_ag_label_vertex";
SELECT COUNT(*) FROM gp7."edges";
SELECT * FROM cypher('gp7', $$MATCH (a)-[e]->(b) RETURN e$$) as (n agtype);
-- SHOULD FAIL, but with an error rather than a crash
SELECT * FROM age_create_barbell_graph('gp8',5,0);
```
On PostgreSQL 18.4, built from source:
* Without the code change, `graph_generation` fails with
`server closed the connection unexpectedly`, and because the crash takes
the
temporary instance down with it, the ten tests that follow fail as well.
* With the code change, `# All 43 tests passed.`
The new cases also confirm the default label is applied: `gp7` gets 10
vertices
under `_ag_label_vertex` (two K5s) and 21 edges (2 × 10 plus the bridge).
### Out of scope
Three other defects in the same function, left alone to keep this change
focused. Happy to open separate issues:
* `if (PG_ARGISNULL(1) && PG_GETARG_INT32(1) < 3)` should use `||`. As
written,
`graph_size` of 1 or 2 passes the check silently.
* `node_properties` and `edge_properties` are accepted but ignored;
`properties` is hardcoded to `create_empty_agtype()`.
* `bridge_size` is validated but unused, as the in-code comment notes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
