db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-01-18 04:58:51 +02:00
Code Issues Releases Activity

Fix missed memory auditing in FUNCTION_LOG_RETURN_VOID().

Browse Source 
9ca492c missed adding auditing to this macro and as a result a few memory leaks have slipped through. Add auditing to the macro to close this hole.

Of the leaks found the only possibly serious one is in blockIncrProcess(), which would leak a PackRead of about eight bytes with every superblock. Since superblocks max out at a few thousand per file this was probably not too bad.

Also change the ordering of auditing in FUNCTION_TEST_RETURN_VOID(). Even though the order does not matter, having it different from the other macros is confusing and looks like an error.
This commit is contained in:
David Steele 2023-06-25 17:36:57 +02:00
parent ecae001653
commit 5cbef3ade2
5 changed files with 45 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/command/backup/blockIncr.c
View File

@ -201,8 +201,8 @@ blockIncrProcess(THIS_VOID, const Buffer *const input, Buffer *const output)
ioWriteClose(this->blockOutWrite);
// Update size and super block size for items already added to the block map
const uint64_t blockOutSize = pckReadU64P(
ioFilterGroupResultP(ioWriteFilterGroup(this->blockOutWrite), SIZE_FILTER_TYPE));
PackRead *const filter = ioFilterGroupResultP(ioWriteFilterGroup(this->blockOutWrite), SIZE_FILTER_TYPE);
const uint64_t blockOutSize = pckReadU64P(filter);
for (unsigned int blockMapIdx = 0; blockMapIdx < lstSize(this->blockOutList); blockMapIdx++)
{
@ -213,6 +213,7 @@ blockIncrProcess(THIS_VOID, const Buffer *const input, Buffer *const output)
blockMapItem->superBlockSize = this->blockOutSize;
}
pckReadFree(filter);
lstFree(this->blockOutList);
// Set to NULL so the super block can be flushed

3
src/common/debug.h
View File

@ -335,6 +335,7 @@ Macros to return function results (or void)
#define FUNCTION_LOG_RETURN_VOID() \
do \
{ \
FUNCTION_TEST_MEM_CONTEXT_AUDIT_END("void"); \
STACK_TRACE_POP(false); \
\
LOG(FUNCTION_LOG_LEVEL(), 0, "=> void"); \
@ -445,8 +446,8 @@ Ignore DEBUG_TEST_TRACE_MACRO if DEBUG is not defined because the underlying fun
{ \
(void)FUNCTION_TEST_BEGIN_exists; /* CHECK for presence of FUNCTION_TEST_BEGIN*() */ \
\
STACK_TRACE_POP(true); \
FUNCTION_TEST_MEM_CONTEXT_AUDIT_END("void"); \
STACK_TRACE_POP(true); \
return; \
} \
while (0)

49
src/storage/posix/write.c
View File

@ -120,30 +120,35 @@ storageWritePosixOpen(THIS_VOID)
// Update user/group owner
if (this->interface.user != NULL || this->interface.group != NULL)
{
const StorageInfo info = storageInterfaceInfoP(this->storage, this->nameTmp, storageInfoLevelDetail, .followLink = true);
ASSERT(info.exists);
uid_t updateUserId = userIdFromName(this->interface.user);
gid_t updateGroupId = groupIdFromName(this->interface.group);
if (updateUserId == (uid_t)-1)
updateUserId = info.userId;
if (updateGroupId == (gid_t)-1)
updateGroupId = info.groupId;
// Continue if one of the owners would be changed
if (updateUserId != info.userId || updateGroupId != info.groupId)
MEM_CONTEXT_TEMP_BEGIN()
{
THROW_ON_SYS_ERROR_FMT(
chown(strZ(this->nameTmp), updateUserId, updateGroupId) == -1, FileOwnerError,
"unable to set ownership for '%s' to %s%s:%s%s from %s[%u]:%s[%u]", strZ(this->nameTmp),
storageWritePosixOpenOwner(this->interface.user, "[none]"),
storageWritePosixOpenOwnerId(this->interface.user, updateUserId),
storageWritePosixOpenOwner(this->interface.group, "[none]"),
storageWritePosixOpenOwnerId(this->interface.group, updateGroupId),
storageWritePosixOpenOwner(info.user, "[unknown]"), info.userId,
storageWritePosixOpenOwner(info.group, "[unknown]"), info.groupId);
const StorageInfo info = storageInterfaceInfoP(
this->storage, this->nameTmp, storageInfoLevelDetail, .followLink = true);
ASSERT(info.exists);
uid_t updateUserId = userIdFromName(this->interface.user);
gid_t updateGroupId = groupIdFromName(this->interface.group);
if (updateUserId == (uid_t)-1)
updateUserId = info.userId;
if (updateGroupId == (gid_t)-1)
updateGroupId = info.groupId;
// Continue if one of the owners would be changed
if (updateUserId != info.userId || updateGroupId != info.groupId)
{
THROW_ON_SYS_ERROR_FMT(
chown(strZ(this->nameTmp), updateUserId, updateGroupId) == -1, FileOwnerError,
"unable to set ownership for '%s' to %s%s:%s%s from %s[%u]:%s[%u]", strZ(this->nameTmp),
storageWritePosixOpenOwner(this->interface.user, "[none]"),
storageWritePosixOpenOwnerId(this->interface.user, updateUserId),
storageWritePosixOpenOwner(this->interface.group, "[none]"),
storageWritePosixOpenOwnerId(this->interface.group, updateGroupId),
storageWritePosixOpenOwner(info.user, "[unknown]"), info.userId,
storageWritePosixOpenOwner(info.group, "[unknown]"), info.groupId);
}
}
MEM_CONTEXT_TEMP_END();
}
FUNCTION_LOG_RETURN_VOID();

2
src/storage/sftp/storage.c
View File

@ -443,6 +443,8 @@ storageSftpRemove(THIS_VOID, const String *const file, const StorageInterfaceRem
}
while (rc == LIBSSH2_ERROR_EAGAIN && waitMore(wait));
waitFree(wait);
if (rc != 0)
{
if (rc == LIBSSH2_ERROR_SFTP_PROTOCOL)

18
test/src/command/test/test.c
View File

@ -24,16 +24,20 @@ cmdTestExec(const String *const command)
ASSERT(cmdTestExecLog != NULL);
ASSERT(command != NULL);
LOG_DETAIL_FMT("exec: %s", strZ(command));
if (system(zNewFmt("%s > %s 2>&1", strZ(command), strZ(cmdTestExecLog))) != 0)
MEM_CONTEXT_TEMP_BEGIN()
{
const Buffer *const buffer = storageGetP(storageNewReadP(storagePosixNewP(FSLASH_STR), cmdTestExecLog));
LOG_DETAIL_FMT("exec: %s", strZ(command));
THROW_FMT(
ExecuteError, "unable to execute: %s > %s 2>&1:\n%s", strZ(command), strZ(cmdTestExecLog),
zNewFmt("\n%s", strZ(strTrim(strNewBuf(buffer)))));
if (system(zNewFmt("%s > %s 2>&1", strZ(command), strZ(cmdTestExecLog))) != 0)
{
const Buffer *const buffer = storageGetP(storageNewReadP(storagePosixNewP(FSLASH_STR), cmdTestExecLog));
THROW_FMT(
ExecuteError, "unable to execute: %s > %s 2>&1:\n%s", strZ(command), strZ(cmdTestExecLog),
zNewFmt("\n%s", strZ(strTrim(strNewBuf(buffer)))));
}
}
MEM_CONTEXT_TEMP_END();
FUNCTION_LOG_RETURN_VOID();
}