Skip to content
Permalink
Browse files

Fix segfault when process-max > 8 for archive-push/archive-get.

The remote list was at most 9 (based on pg[1-8]-* max index) so anything over 8 wrote into unallocated memory.

The remote for the main process is (currently) stored in position zero so do the same for remotes started from locals, since there should only be one.  The main process will need to start more remotes in the future which is why there is extra space.

Reported by Jens Wilke.
  • Loading branch information...
dwsteele committed Apr 29, 2019
1 parent c935b1c commit d0c296bd5b67966fd35fd9f70c25788dd6de0c34
Showing with 18 additions and 5 deletions.
  1. +11 −0 doc/xml/release.xml
  2. +5 −3 src/protocol/helper.c
  3. +2 −2 test/src/module/protocol/protocolTest.c
@@ -14,6 +14,16 @@
<release-list>
<release date="XXXX-XX-XX" version="2.14dev" title="UNDER DEVELOPMENT">
<release-core-list>
<release-bug-list>
<release-item>
<release-item-contributor-list>
<release-item-ideator id="jens.wilke"/>
</release-item-contributor-list>

<p>Fix segfault when <br-option>process-max</br-option> > 8 for <cmd>archive-push</cmd>/<cmd>archive-get</cmd>.</p>
</release-item>
</release-bug-list>

<release-improvement-list>
<release-item>
<p>Improve performance of non-blocking reads by using maximum buffer size.</p>
@@ -6975,6 +6985,7 @@

<contributor id="jens.wilke">
<contributor-name-display>Jens Wilke</contributor-name-display>
<contributor-id type="github">jwpit</contributor-id>
</contributor>

<contributor id="jesper.st.john">
@@ -274,16 +274,18 @@ protocolRemoteGet(ProtocolStorageType protocolStorageType)
}

// Determine protocol id for the remote. If the process option is set then use that since we want to remote protocol id to
// match the local protocol id. Otherwise set to 0 since the remote is being started from a main process.
// match the local protocol id (but we'll still save it in position 0 or we'd need to allocated up to process-max slots).
// Otherwise set to 0 since the remote is being started from a main process.
unsigned int protocolId = 0;
unsigned int protocolIdx = 0;

if (cfgOptionTest(cfgOptProcess))
protocolId = cfgOptionUInt(cfgOptProcess);

ASSERT(protocolId < protocolHelper.clientRemoteSize);
CHECK(protocolIdx < protocolHelper.clientRemoteSize);

// Create protocol object
ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientRemote[protocolId];
ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientRemote[protocolIdx];

if (protocolHelperClient->client == NULL)
{
@@ -685,7 +685,7 @@ testRun(void)
strLstAdd(argList, strNewFmt("--config=%s/pgbackrest.conf", testPath()));
strLstAddZ(argList, "--repo1-host=localhost");
strLstAdd(argList, strNewFmt("--repo1-path=%s", testPath()));
strLstAddZ(argList, "--process=4");
strLstAddZ(argList, "--process=999");
strLstAddZ(argList, "--command=archive-get");
strLstAddZ(argList, "--host-id=1");
strLstAddZ(argList, "--type=db");
@@ -694,7 +694,7 @@ testRun(void)

TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "acbd", "check cipher pass before");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo), "get remote protocol");
TEST_RESULT_PTR(protocolHelper.clientRemote[4].client, client, "check position in cache");
TEST_RESULT_PTR(protocolHelper.clientRemote[0].client, client, "check position in cache");
TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "acbd", "check cipher pass after");

TEST_RESULT_VOID(protocolFree(), "free remote protocol objects");

0 comments on commit d0c296b

Please sign in to comment.
You can’t perform that action at this time.