Engine: server bulk ban check bug fixes

If the code was ran in the main thread, it would never fire the actual check logic. This patch changed the code so it would always fire from the start of the frame. Also fixed a memory leak that occurred when passing in an external banned list to SV_CheckClientsForBan (previously, 'SV_CheckForBan'); the code always allocated a new banned list, but only freed it if no external banned list was provided. This patch changed the code so it only allocates it if no external one is provided.
This commit is contained in:
Kawe Mazidjatari 2024-03-10 02:13:15 +01:00
parent f9559b609d
commit 6f962a8e2b
3 changed files with 51 additions and 54 deletions

View File

@ -184,7 +184,10 @@ CClient* CServer::ConnectClient(CServer* pServer, user_creds_s* pChallenge)
{
if (!pClient->GetNetChan()->GetRemoteAddress().IsLoopback())
{
std::thread th(SV_IsClientBanned, pClient, string(pszAddresBuffer), nNucleusID, string(pszPersonaName), nPort);
const string addressBufferCopy(pszAddresBuffer);
const string personaNameCopy(pszPersonaName);
std::thread th(SV_CheckForBanAndDisconnect, pClient, addressBufferCopy, nNucleusID, personaNameCopy, nPort);
th.detach();
}
}

View File

@ -18,78 +18,71 @@
//-----------------------------------------------------------------------------
// Purpose: checks if particular client is banned on the comp server
//-----------------------------------------------------------------------------
void SV_IsClientBanned(CClient* pClient, const string& svIPAddr,
void SV_CheckForBanAndDisconnect(CClient* const pClient, const string& svIPAddr,
const NucleusID_t nNucleusID, const string& svPersonaName, const int nPort)
{
Assert(pClient != nullptr);
string svError;
bool bCompBanned = g_MasterServer.CheckForBan(svIPAddr, nNucleusID, svPersonaName, svError);
const bool bCompBanned = g_MasterServer.CheckForBan(svIPAddr, nNucleusID, svPersonaName, svError);
if (bCompBanned)
{
if (!ThreadInMainThread())
{
g_TaskQueue.Dispatch([pClient, svError, svIPAddr, nNucleusID, nPort]
g_TaskQueue.Dispatch([pClient, svError, svIPAddr, nNucleusID, nPort]
{
// Make sure client isn't already disconnected,
// and that if there is a valid netchannel, that
// it hasn't been taken by a different client by
// the time this task is getting executed.
const CNetChan* const pChan = pClient->GetNetChan();
if (pChan && pClient->GetNucleusID() == nNucleusID)
{
// Make sure client isn't already disconnected,
// and that if there is a valid netchannel, that
// it hasn't been taken by a different client by
// the time this task is getting executed.
CNetChan* pChan = pClient->GetNetChan();
if (pChan && pClient->GetNucleusID() == nNucleusID)
{
int nUserID = pClient->GetUserID();
const int nUserID = pClient->GetUserID();
pClient->Disconnect(Reputation_t::REP_MARK_BAD, svError.c_str());
Warning(eDLL_T::SERVER, "Removed client '[%s]:%i' from slot #%i ('%llu' is banned globally!)\n",
svIPAddr.c_str(), nPort, nUserID, nNucleusID);
}
}, 0);
}
pClient->Disconnect(Reputation_t::REP_MARK_BAD, svError.c_str());
Warning(eDLL_T::SERVER, "Removed client '[%s]:%i' from slot #%i ('%llu' is banned globally!)\n",
svIPAddr.c_str(), nPort, nUserID, nNucleusID);
}
}, 0);
}
}
//-----------------------------------------------------------------------------
// Purpose: checks if particular client is banned on the master server
//-----------------------------------------------------------------------------
void SV_ProcessBulkCheck(const CBanSystem::BannedList_t* pBannedVec, const bool bDelete)
void SV_ProcessBulkCheck(const CBanSystem::BannedList_t* const pBannedVec)
{
CBanSystem::BannedList_t* outBannedVec = new CBanSystem::BannedList_t();
CBanSystem::BannedList_t* const outBannedVec = new CBanSystem::BannedList_t();
g_MasterServer.GetBannedList(*pBannedVec, *outBannedVec);
// Caller wants to destroy the vector.
if (bDelete)
{
delete pBannedVec;
}
if (!ThreadInMainThread())
{
g_TaskQueue.Dispatch([outBannedVec]
{
SV_CheckForBan(outBannedVec, true);
}, 0);
}
g_TaskQueue.Dispatch([outBannedVec]
{
SV_CheckClientsForBan(outBannedVec);
delete outBannedVec;
}, 0);
}
//-----------------------------------------------------------------------------
// Purpose: creates a snapshot of the currently connected clients
// Input : *pBannedVec - if passed, will check for bans and kick the clients
// bDelete - if set, will delete the passed in vector
//-----------------------------------------------------------------------------
void SV_CheckForBan(const CBanSystem::BannedList_t* pBannedVec /*= nullptr*/, const bool bDelete /*= false*/)
void SV_CheckClientsForBan(const CBanSystem::BannedList_t* const pBannedVec /*= nullptr*/)
{
Assert(ThreadInMainThread());
CBanSystem::BannedList_t* bannedVec = new CBanSystem::BannedList_t;
CBanSystem::BannedList_t* bannedVec = !pBannedVec
? new CBanSystem::BannedList_t
: nullptr;
for (int c = 0; c < g_ServerGlobalVariables->m_nMaxClients; c++) // Loop through all possible client instances.
{
CClient* pClient = g_pServer->GetClient(c);
CClient* const pClient = g_pServer->GetClient(c);
if (!pClient)
continue;
CNetChan* pNetChan = pClient->GetNetChan();
const CNetChan* const pNetChan = pClient->GetNetChan();
if (!pNetChan)
continue;
@ -99,13 +92,13 @@ void SV_CheckForBan(const CBanSystem::BannedList_t* pBannedVec /*= nullptr*/, co
if (pNetChan->GetRemoteAddress().IsLoopback())
continue;
const char* szIPAddr = pNetChan->GetAddress(true);
const char* const szIPAddr = pNetChan->GetAddress(true);
const NucleusID_t nNucleusID = pClient->GetNucleusID();
// If no banned list was provided, build one with all clients
// on the server. This will be used for bulk checking so live
// bans could be performed, as this function is called periodically.
if (!pBannedVec)
if (bannedVec)
bannedVec->AddToTail(CBanSystem::Banned_t(szIPAddr, nNucleusID));
else
{
@ -128,19 +121,20 @@ void SV_CheckForBan(const CBanSystem::BannedList_t* pBannedVec /*= nullptr*/, co
}
}
// Caller wants to destroy the vector.
if (bDelete && pBannedVec)
if (bannedVec && !bannedVec->IsEmpty())
{
delete pBannedVec;
}
std::thread bulkCheck([bannedVec]()
{
SV_ProcessBulkCheck(bannedVec);
delete bannedVec;
});
if (!pBannedVec && !bannedVec->IsEmpty())
{
std::thread(&SV_ProcessBulkCheck, bannedVec, true).detach();
bulkCheck.detach();
}
else
else if (bannedVec)
{
delete bannedVec;
bannedVec = nullptr;
}
}
@ -169,7 +163,7 @@ bool SV_ActivateServer()
return v_SV_ActivateServer();
}
void SV_BroadcastVoiceData(CClient* cl, int nBytes, char* data)
void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data)
{
if (!sv_voiceenable->GetBool())
return;

View File

@ -28,9 +28,9 @@ inline bool IsDedicated()
void SV_InitGameDLL();
void SV_ShutdownGameDLL();
bool SV_ActivateServer();
void SV_BroadcastVoiceData(CClient* cl, int nBytes, char* data);
void SV_IsClientBanned(CClient* pClient, const string& svIPAddr, const NucleusID_t nNucleusID, const string& svPersonaName, const int nPort);
void SV_CheckForBan(const CBanSystem::BannedList_t* pBannedVec = nullptr, const bool bDelete = false);
void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data);
void SV_CheckForBanAndDisconnect(CClient* const pClient, const string& svIPAddr, const NucleusID_t nNucleusID, const string& svPersonaName, const int nPort);
void SV_CheckClientsForBan(const CBanSystem::BannedList_t* const pBannedVec = nullptr);
///////////////////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////////////////