Skip to content

Commit 68eda9a

Browse files
authored
Fix memory management issue in monitor_register_node. (#590)
We should refrain from calling strlcpy() on one of the function arguments.
1 parent 6079caf commit 68eda9a

3 files changed

Lines changed: 10 additions & 8 deletions

File tree

src/bin/pg_autoctl/keeper.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,6 +1352,9 @@ keeper_register_and_init(Keeper *keeper, NodeState initialState)
13521352
(void) pg_usleep(sleepTimeMs * 1000);
13531353
}
13541354

1355+
/* we might have been assigned a new name */
1356+
strlcpy(config->name, assignedState.name, sizeof(config->name));
1357+
13551358
/* initialize FSM state from monitor's answer */
13561359
log_info("Writing keeper state file at \"%s\"", config->pathnames.state);
13571360

src/bin/pg_autoctl/monitor.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ typedef struct NodeAddressArrayParseContext
5151
typedef struct MonitorAssignedStateParseContext
5252
{
5353
char sqlstate[SQLSTATE_LENGTH];
54-
char name[_POSIX_HOST_NAME_MAX];
5554
MonitorAssignedState *assignedState;
5655
bool parsedOK;
5756
} MonitorAssignedStateParseContext;
@@ -801,7 +800,7 @@ monitor_register_node(Monitor *monitor, char *formation,
801800
};
802801
const char *paramValues[11];
803802
MonitorAssignedStateParseContext parseContext =
804-
{ { 0 }, { 0 }, assignedState, false };
803+
{ { 0 }, assignedState, false };
805804
const char *nodeStateString = NodeStateToString(initialState);
806805

807806
paramValues[0] = formation;
@@ -861,12 +860,9 @@ monitor_register_node(Monitor *monitor, char *formation,
861860
return false;
862861
}
863862

864-
/* update the caller's name with the result from the monitor */
865-
strlcpy(name, parseContext.name, _POSIX_HOST_NAME_MAX);
866-
867863
log_info("Registered node %d (%s:%d) with name \"%s\" in formation \"%s\", "
868864
"group %d, state \"%s\"",
869-
assignedState->nodeId, host, port, name,
865+
assignedState->nodeId, host, port, assignedState->name,
870866
formation, assignedState->groupId,
871867
NodeStateToString(assignedState->state));
872868

@@ -897,7 +893,7 @@ monitor_node_active(Monitor *monitor,
897893
};
898894
const char *paramValues[7];
899895
MonitorAssignedStateParseContext parseContext =
900-
{ { 0 }, { 0 }, assignedState, false };
896+
{ { 0 }, assignedState, false };
901897
const char *nodeStateString = NodeStateToString(currentState);
902898

903899
paramValues[0] = formation;
@@ -1609,7 +1605,9 @@ parseNodeState(void *ctx, PGresult *result)
16091605
if (PQnfields(result) == 6)
16101606
{
16111607
value = PQgetvalue(result, 0, 5);
1612-
strlcpy(context->name, value, _POSIX_HOST_NAME_MAX);
1608+
strlcpy(context->assignedState->name,
1609+
value,
1610+
sizeof(context->assignedState->name));
16131611
}
16141612

16151613
/* if we reach this line, then we're good. */

src/bin/pg_autoctl/monitor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ typedef struct Monitor
2626

2727
typedef struct MonitorAssignedState
2828
{
29+
char name[_POSIX_HOST_NAME_MAX];
2930
int nodeId;
3031
int groupId;
3132
NodeState state;

0 commit comments

Comments
 (0)