From daf34c3e341dbd817dca9310bfe5b5f24d9a20c4 Mon Sep 17 00:00:00 2001 From: RaphaelIT7 <64648134+RaphaelIT7@users.noreply.github.com> Date: Sun, 16 Nov 2025 23:36:27 +0100 Subject: [PATCH] engine: make Filter_ShouldDiscard threadsafe --- engine/sv_filter.cpp | 174 +++++++++++++++++++++---------------- public/tier0/threadtools.h | 47 ++++++++++ 2 files changed, 144 insertions(+), 77 deletions(-) diff --git a/engine/sv_filter.cpp b/engine/sv_filter.cpp index 19ac1fb5e..0d92aaf96 100644 --- a/engine/sv_filter.cpp +++ b/engine/sv_filter.cpp @@ -26,6 +26,11 @@ static ConVar sv_filterban( "sv_filterban", "1", 0, "Set packet filtering by IP CUtlVector< ipfilter_t > g_IPFilters; CUtlVector< userfilter_t > g_UserFilters; +#if defined(WIN32) || defined(_WIN32) +CThreadSpinRWLock g_IPFilterMutex; +#else +CThreadRWLock g_IPFilterMutex; +#endif #define BANNED_IP_FILENAME "banned_ip.cfg" #define BANNED_USER_FILENAME "banned_user.cfg" @@ -45,6 +50,7 @@ void Filter_SendBan( const netadr_t& adr ) // Purpose: Checks an IP address to see if it is banned // Input : *adr - // Output : bool +// RaphaelIT7: This function is fully threadsafe in case the network layer gets threaded //----------------------------------------------------------------------------- bool Filter_ShouldDiscard( const netadr_t& adr ) { @@ -57,6 +63,8 @@ bool Filter_ShouldDiscard( const netadr_t& adr ) unsigned in = *(unsigned *)&adr.ip[0]; + AUTO_LOCK_READ( g_IPFilterMutex ); + // Handle timeouts for ( intp i = g_IPFilters.Count() - 1 ; i >= 0 ; i--) { @@ -176,41 +184,47 @@ static void Filter_Add_f( const CCommand& args ) return; intp i; - for (i=0 ; i g_IPFilters.Count() ) + AUTO_LOCK_WRITE( g_IPFilterMutex ); + + // if no "." in the string we'll assume it's a slot number + if ( !V_strstr( args[1], "." ) ) { - ConMsg( "removeip: Invalid slot %i\n", slot ); - return; - } + int slot = V_atoi( args[1] ); + if ( slot <= 0 || slot > g_IPFilters.Count() ) + { + ConMsg( "removeip: Invalid slot %i\n", slot ); + return; + } - // array access is zero based - byte b[4]; - memcpy( b, &g_IPFilters[--slot].compare, sizeof(b) ); + // array access is zero based + byte b[4]; + memcpy( b, &g_IPFilters[--slot].compare, sizeof(b) ); - g_IPFilters.Remove( slot ); + g_IPFilters.Remove( slot ); - char szIP[32]; - V_sprintf_safe( szIP, "%i.%i.%i.%i", b[0], b[1], b[2], b[3] ); - // Tell server operator - ConMsg( "removeip: Filter removed for %s, IP %s\n", args[1], szIP ); + V_sprintf_safe( removedIP, "%i.%i.%i.%i", b[0], b[1], b[2], b[3] ); - // send an event - IGameEvent *event = g_GameEventManager.CreateEvent( "server_removeban" ); - if ( event ) - { - event->SetString( "networkid", "" ); - event->SetString( "ip", szIP ); - event->SetString( "by", ( cmd_source == src_command ) ? "Console" : host_client->m_Name ); - - g_GameEventManager.FireEvent( event ); + removed = true; + eventIP = removedIP; + return; } - return; - } - - ipfilter_t f; - if ( !Filter_ConvertString( args[1], &f ) ) - return; + ipfilter_t f; + if ( !Filter_ConvertString( args[1], &f ) ) + return; - for ( intp i = 0 ; i < g_IPFilters.Count() ; i++ ) - { - if ( ( g_IPFilters[i].mask == f.mask ) && - ( g_IPFilters[i].compare == f.compare ) ) + for ( intp i = 0 ; i < g_IPFilters.Count() ; i++ ) { - g_IPFilters.Remove(i); - - ConMsg( "removeip: filter removed for %s\n", args[1] ); - - // send an event - IGameEvent *event = g_GameEventManager.CreateEvent( "server_removeban" ); - if ( event ) + if ( ( g_IPFilters[i].mask == f.mask ) && + ( g_IPFilters[i].compare == f.compare ) ) { - event->SetString( "networkid", "" ); - event->SetString( "ip", args[1] ); - event->SetString( "by", ( cmd_source == src_command ) ? "Console" : host_client->m_Name ); + g_IPFilters.Remove(i); - g_GameEventManager.FireEvent( event ); + removed = true; + eventIP = arg; + break; } - - return; } } - ConMsg( "removeip: Couldn't find %s\n", args[1] ); + if ( !removed ) + { + ConMsg( "removeip: Couldn't find %s\n", arg ); + return; + } + + ConMsg( "removeip: Filter removed for %s\n", eventIP ); + + IGameEvent *event = g_GameEventManager.CreateEvent( "server_removeban" ); + if ( event ) + { + event->SetString( "networkid", "" ); + event->SetString( "ip", eventIP ); + event->SetString( "by", ( cmd_source == src_command ) ? "Console" : host_client->m_Name ); + g_GameEventManager.FireEvent( event ); + } } @@ -363,6 +380,7 @@ CON_COMMAND( removeip, "Remove an IP address from the ban list." ) //----------------------------------------------------------------------------- CON_COMMAND( listip, "List IP addresses on the ban list." ) { + AUTO_LOCK_READ( g_IPFilterMutex ); const intp count = g_IPFilters.Count(); if ( !count ) { @@ -407,6 +425,8 @@ CON_COMMAND( writeip, "Save the ban list to " BANNED_IP_FILENAME "." ) return; } + AUTO_LOCK_READ( g_IPFilterMutex ); + byte b[4]; for ( const auto &ipf : g_IPFilters ) { diff --git a/public/tier0/threadtools.h b/public/tier0/threadtools.h index e64bc8bdc..273217dcc 100644 --- a/public/tier0/threadtools.h +++ b/public/tier0/threadtools.h @@ -1237,6 +1237,53 @@ class PLATFORM_CLASS CThreadRWLock int m_nPendingReaders{ 0 }; }; +template +class CAutoReadLockT +{ +public: + FORCEINLINE CAutoReadLockT(RWLOCK_TYPE &lock) + : m_lock(lock) + { + m_lock.LockForRead(); + } + + FORCEINLINE ~CAutoReadLockT() + { + m_lock.UnlockRead(); + } + +private: + RWLOCK_TYPE &m_lock; + + CAutoReadLockT(const CAutoReadLockT &) = delete; + CAutoReadLockT &operator=(const CAutoReadLockT &) = delete; +}; + +template +class CAutoWriteLockT +{ +public: + FORCEINLINE CAutoWriteLockT(RWLOCK_TYPE &lock) + : m_lock(lock) + { + m_lock.LockForWrite(); + } + + FORCEINLINE ~CAutoWriteLockT() + { + m_lock.UnlockWrite(); + } + +private: + RWLOCK_TYPE &m_lock; + + CAutoWriteLockT(const CAutoWriteLockT &) = delete; + CAutoWriteLockT &operator=(const CAutoWriteLockT &) = delete; +}; + +#define AUTO_LOCK_READ(rwlock) CAutoReadLockT UNIQUE_ID(rwlock) +#define AUTO_LOCK_WRITE(rwlock) CAutoWriteLockT UNIQUE_ID(rwlock) + //----------------------------------------------------------------------------- // // CThreadSpinRWLock