Skip to content

Commit ddd89a8

Browse files
enjoy-binbinrjd15372
authored andcommitted
Fix memory leak in saveSnapshotToConnectionSockets (valkey-io#2503)
We now pass in rdbSnapshotOptions options in this function, and options.conns is now malloc'ed in the caller side, so we need to zfree it when returning early due to an error. Previously, conns was malloc'ed after the error handling, so we don't have this. Introduced in valkey-io#1949. --------- Signed-off-by: Binbin <[email protected]>
1 parent becd43d commit ddd89a8

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

src/rdb.c

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3591,24 +3591,34 @@ void killRDBChild(void) {
35913591
/* Save snapshot to the provided connections, spawning a child process and
35923592
* running the provided function.
35933593
*
3594-
* Connections array provided will be freed after the save is completed, and
3595-
* should not be freed by the caller. */
3594+
* The connections array (the conns field in the rdbSnapshotOptions) is a
3595+
* heap-allocated array that will be freed by this function and shall not be
3596+
* freed by the caller. */
35963597
int saveSnapshotToConnectionSockets(rdbSnapshotOptions options) {
35973598
pid_t childpid;
35983599
int pipefds[2], rdb_pipe_write = -1, safe_to_exit_pipe = -1;
3599-
if (hasActiveChildProcess()) return C_ERR;
3600+
if (hasActiveChildProcess()) {
3601+
zfree(options.conns);
3602+
return C_ERR;
3603+
}
36003604
serverAssert(server.rdb_pipe_read == -1 && server.rdb_child_exit_pipe == -1);
36013605

36023606
/* Even if the previous fork child exited, don't start a new one until we
36033607
* drained the pipe. */
3604-
if (server.rdb_pipe_conns) return C_ERR;
3608+
if (server.rdb_pipe_conns) {
3609+
zfree(options.conns);
3610+
return C_ERR;
3611+
}
36053612

36063613
if (options.use_pipe) {
36073614
/* Before to fork, create a pipe that is used to transfer the rdb bytes to
36083615
* the parent, we can't let it write directly to the sockets, since in case
36093616
* of TLS we must let the parent handle a continuous TLS state when the
36103617
* child terminates and parent takes over. */
3611-
if (anetPipe(pipefds, O_NONBLOCK, 0) == -1) return C_ERR;
3618+
if (anetPipe(pipefds, O_NONBLOCK, 0) == -1) {
3619+
zfree(options.conns);
3620+
return C_ERR;
3621+
}
36123622
server.rdb_pipe_read = pipefds[0]; /* read end */
36133623
rdb_pipe_write = pipefds[1]; /* write end */
36143624

@@ -3617,6 +3627,7 @@ int saveSnapshotToConnectionSockets(rdbSnapshotOptions options) {
36173627
if (anetPipe(pipefds, 0, 0) == -1) {
36183628
close(rdb_pipe_write);
36193629
close(server.rdb_pipe_read);
3630+
zfree(options.conns);
36203631
return C_ERR;
36213632
}
36223633
safe_to_exit_pipe = pipefds[0]; /* read end */

src/rdb.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ enum RdbType {
120120
typedef int (*ChildSnapshotFunc)(int req, rio *rdb, void *privdata);
121121
typedef struct rdbSnapshotOptions {
122122
int connsnum; /* Number of connections. */
123-
connection **conns; /* Connections to send the snapshot to. */
123+
connection **conns; /* A heap-allocated array of connections to send the snapshot to. */
124124
int use_pipe; /* Use pipe to send the snapshot. */
125125
int req; /* See REPLICA_REQ_* in server.h. */
126126
int skip_checksum; /* Skip checksum when sending the snapshot. */

0 commit comments

Comments
 (0)