issue-1794: fixed use-after-free in fs search by alias, not making a …

…new THashMap upon each alias search, removed incorrect TABLET_VERIFY in TIndexTabletActor::CompleteTx_CreateSession (#1816)

* issue-1794: fixed use-after-free in fs search by alias, not making a new THashMap upon each alias search, removed incorrect TABLET_VERIFY in TIndexTabletActor::CompleteTx_CreateSession

* issue-1794: fixed use-after-free in fs search by alias, not making a new THashMap upon each alias search, removed incorrect TABLET_VERIFY in TIndexTabletActor::CompleteTx_CreateSession - fixed comment

cloud/filestore/libs/storage/core/config.cpp

@@ -16,9 +16,7 @@ namespace {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-// Type alias is used here because using THashMap<TString, TString> inside macro
-// will mess it up because of the comma in the type.
-using TAliasMap = THashMap<TString, TString>;
+using TAliases = NProto::TStorageConfig::TFilestoreAliases;
 
 #define FILESTORE_STORAGE_CONFIG(xxx)                                          \
     xxx(SchemeShardDir,                TString,   "/Root"                     )\
@@ -182,11 +180,13 @@ using TAliasMap = THashMap<TString, TString>;
     xxx(BlobCompressionRate,             ui32,                  0             )\
     xxx(BlobCompressionCodec,            TString,               "lz4"         )\
                                                                                \
-    xxx(FilestoreAliases,                TAliasMap,             {}            )\
-                                                                               \
     xxx(MaxZeroCompactionRangesToDeletePerTx,           ui32,      10000      )\
 // FILESTORE_STORAGE_CONFIG
 
+#define FILESTORE_STORAGE_CONFIG_REF(xxx)                                      \
+    xxx(FilestoreAliases,                TAliases,              {}            )\
+// FILESTORE_STORAGE_CONFIG_REF
+
 #define FILESTORE_DECLARE_CONFIG(name, type, value)                            \
     Y_DECLARE_UNUSED static const type Default##name = value;                  \
 // FILESTORE_DECLARE_CONFIG
@@ -210,7 +210,7 @@ bool IsEmpty(const NCloud::NProto::TCertificate& value)
 }
 
 template <>
-bool IsEmpty(const NProto::TStorageConfig::TFilestoreAliases& value)
+bool IsEmpty(const TAliases& value)
 {
     return value.GetEntries().empty();
 }
@@ -227,16 +227,6 @@ TCertificate ConvertValue(const NCloud::NProto::TCertificate& value)
     return {value.GetCertFile(), value.GetCertPrivateKeyFile()};
 }
 
-template <>
-TAliasMap ConvertValue(const NProto::TStorageConfig::TFilestoreAliases& value)
-{
-    TAliasMap result;
-    for (const auto& entry: value.GetEntries()) {
-        result[entry.GetAlias()] = entry.GetFsId();
-    }
-    return result;
-}
-
 template <>
 TDuration ConvertValue<TDuration, ui32>(const ui32& value)
 {
@@ -274,11 +264,11 @@ void DumpImpl(const TCertificate& value, IOutputStream& os)
 }
 
 template <>
-void DumpImpl(const TAliasMap& value, IOutputStream& os)
+void DumpImpl(const TAliases& value, IOutputStream& os)
 {
     os << "{ ";
-    for (const auto& [alias, fsId]: value) {
-        os << alias << ": " << fsId << ", ";
+    for (const auto& x: value.GetEntries()) {
+        os << x.GetAlias() << ": " << x.GetFsId() << ", ";
     }
     os << " }";
 }
@@ -299,6 +289,17 @@ FILESTORE_STORAGE_CONFIG(FILESTORE_CONFIG_GETTER)
 
 #undef FILESTORE_CONFIG_GETTER
 
+#define FILESTORE_CONFIG_GETTER_REF(name, type, ...)                           \
+const type& TStorageConfig::Get##name() const                                  \
+{                                                                              \
+    return ProtoConfig.Get##name();                                            \
+}                                                                              \
+// FILESTORE_CONFIG_GETTER_REF
+
+FILESTORE_STORAGE_CONFIG_REF(FILESTORE_CONFIG_GETTER_REF)
+
+#undef FILESTORE_CONFIG_GETTER_REF
+
 void TStorageConfig::Dump(IOutputStream& out) const
 {
 #define FILESTORE_DUMP_CONFIG(name, ...)                                       \
@@ -308,6 +309,7 @@ void TStorageConfig::Dump(IOutputStream& out) const
 // FILESTORE_DUMP_CONFIG
 
     FILESTORE_STORAGE_CONFIG(FILESTORE_DUMP_CONFIG);
+    FILESTORE_STORAGE_CONFIG_REF(FILESTORE_DUMP_CONFIG);
 
 #undef FILESTORE_DUMP_CONFIG
 }
@@ -325,6 +327,7 @@ void TStorageConfig::DumpHtml(IOutputStream& out) const
         TABLE_CLASS("table table-condensed") {
             TABLEBODY() {
                 FILESTORE_STORAGE_CONFIG(FILESTORE_DUMP_CONFIG);
+                FILESTORE_STORAGE_CONFIG_REF(FILESTORE_DUMP_CONFIG);
             }
         }
     }
@@ -349,6 +352,7 @@ void TStorageConfig::DumpOverridesHtml(IOutputStream& out) const
         TABLE_CLASS("table table-condensed") {
             TABLEBODY() {
                 FILESTORE_STORAGE_CONFIG(FILESTORE_DUMP_CONFIG);
+                FILESTORE_STORAGE_CONFIG_REF(FILESTORE_DUMP_CONFIG);
             }
         }
     }
@@ -394,4 +398,16 @@ const NProto::TStorageConfig& TStorageConfig::GetStorageConfigProto() const
     return ProtoConfig;
 }
 
+const TString* TStorageConfig::FindFileSystemIdByAlias(
+    const TString& alias) const
+{
+    const auto& entries = GetFilestoreAliases().GetEntries();
+    for (const auto& entry: entries) {
+        if (entry.GetAlias() == alias) {
+            return &entry.GetFsId();
+        }
+    }
+    return nullptr;
+}
+
 }   // namespace NCloud::NFileStore::NStorage

cloud/filestore/libs/storage/core/config.h

@@ -240,7 +240,8 @@ class TStorageConfig
 
     const NProto::TStorageConfig& GetStorageConfigProto() const;
 
-    THashMap<TString, TString> GetFilestoreAliases() const;
+    const NProto::TStorageConfig::TFilestoreAliases& GetFilestoreAliases() const;
+    const TString* FindFileSystemIdByAlias(const TString& alias) const;
 };
 
 }   // namespace NCloud::NFileStore::NStorage

cloud/filestore/libs/storage/service/service_actor_createsession.cpp

@@ -653,9 +653,10 @@ void TStorageServiceActor::HandleCreateSession(
 
     const auto& clientId = GetClientId(msg->Record);
     TString fileSystemId = msg->Record.GetFileSystemId();
-    auto it = StorageConfig->GetFilestoreAliases().find(fileSystemId);
-    if (it != StorageConfig->GetFilestoreAliases().end()) {
-        fileSystemId = it->second;
+    if (const auto* realId =
+            StorageConfig->FindFileSystemIdByAlias(fileSystemId))
+    {
+        fileSystemId = *realId;
     }
     const auto& checkpointId = msg->Record.GetCheckpointId();
     auto originFqdn = GetOriginFqdn(msg->Record);

cloud/filestore/libs/storage/tablet/tablet_actor_createsession.cpp

@@ -444,7 +444,19 @@ void TIndexTabletActor::CompleteTx_CreateSession(
     }
 
     auto* session = FindSession(args.SessionId);
-    TABLET_VERIFY(session);
+    if (!session) {
+        auto message = TStringBuilder() << "Session " << args.SessionId
+            << " destroyed during creation";
+        LOG_WARN(ctx, TFileStoreComponents::TABLET,
+            "%s %s",
+            LogTag.c_str(),
+            message.c_str());
+
+        auto response =
+            std::make_unique<TResponse>(MakeError(E_REJECTED, message));
+        NCloud::Reply(ctx, *args.RequestInfo, std::move(response));
+        return;
+    }
 
     auto response = std::make_unique<TResponse>(args.Error);
     response->Record.SetSessionId(std::move(args.SessionId));

cloud/filestore/libs/storage/tablet_proxy/tablet_proxy_actor.cpp

@@ -320,9 +320,8 @@ void TIndexTabletProxyActor::HandleRequest(
 
     TString fileSystemId = GetFileSystemId(*msg);
     // Some filestore names can point to another filestore, set by storage config
-    auto it = Config->GetFilestoreAliases().find(fileSystemId);
-    if (it != Config->GetFilestoreAliases().end()) {
-        fileSystemId = it->second;
+    if (const auto* realId = Config->FindFileSystemIdByAlias(fileSystemId)) {
+        fileSystemId = *realId;
     }
 
     TConnection& conn = CreateConnection(fileSystemId);