From f33d06b8739795adf875fb06d30a9a786ec19b21 Mon Sep 17 00:00:00 2001 From: Gary Talent Date: Fri, 20 Dec 2024 00:28:00 -0600 Subject: [PATCH] [keel] Fix a use after free, cleanup --- src/olympic/keel/include/keel/media.hpp | 53 ++++++++++++++----------- src/olympic/keel/src/media.cpp | 24 +++++------ 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/olympic/keel/include/keel/media.hpp b/src/olympic/keel/include/keel/media.hpp index 6d6a7be0..17646fd3 100644 --- a/src/olympic/keel/include/keel/media.hpp +++ b/src/olympic/keel/include/keel/media.hpp @@ -33,7 +33,7 @@ ox::Result getPreloadAddr(keel::Context &ctx, ox::FileAddress const ox::Result getPreloadAddr(keel::Context &ctx, ox::StringViewCR path) noexcept; -void createUuidMapping(Context &ctx, ox::StringView filePath, ox::UUID const&uuid) noexcept; +void createUuidMapping(Context &ctx, ox::StringViewCR filePath, ox::UUID const&uuid) noexcept; ox::Error buildUuidMap(Context &ctx) noexcept; @@ -41,17 +41,17 @@ ox::Result pathToUuid(Context &ctx, ox::StringViewCR path) noexcept; ox::Result getUuid(Context &ctx, ox::FileAddress const&fileAddr) noexcept; -ox::Result getUuid(Context &ctx, ox::StringView path) noexcept; +ox::Result getUuid(Context &ctx, ox::StringViewCR path) noexcept; ox::Result getPath(Context &ctx, ox::FileAddress const&fileAddr) noexcept; -ox::Result getPath(Context &ctx, ox::CStringView fileId) noexcept; +ox::Result getPath(Context &ctx, ox::CStringViewCR fileId) noexcept; -constexpr ox::Result uuidUrlToUuid(ox::StringView uuidUrl) noexcept { +constexpr ox::Result uuidUrlToUuid(ox::StringViewCR uuidUrl) noexcept { return ox::UUID::fromString(substr(uuidUrl, 7)); } -ox::Result uuidUrlToPath(Context &ctx, ox::StringView uuid) noexcept; +ox::Result uuidUrlToPath(Context &ctx, ox::StringViewCR uuid) noexcept; ox::Result uuidToPath(Context &ctx, ox::StringViewCR uuid) noexcept; @@ -62,7 +62,7 @@ ox::Result uuidToPath(Context &ctx, ox::UUID const&uuid) noexce namespace detail { template constexpr auto makeLoader(Context &ctx) { - return [&ctx](ox::StringView assetId) -> ox::Result { + return [&ctx](ox::StringViewCR assetId) -> ox::Result { OX_REQUIRE(p, ctx.uuidToPath.at(assetId)); OX_REQUIRE(buff, ctx.rom->read(*p)); auto [obj, err] = readAsset(buff); @@ -80,27 +80,34 @@ constexpr auto makeLoader(Context &ctx) { template ox::Result> readObjFile( keel::Context &ctx, - ox::StringView assetId, - bool forceLoad) noexcept { + ox::StringViewCR assetId, + bool const forceLoad) noexcept { + static constexpr auto load = []( + keel::Context &ctx, + ox::StringViewCR assetId, + bool forceLoad) -> ox::Result> { + if (forceLoad) { + ctx.assetManager.initTypeManager(detail::makeLoader, ctx); + return ctx.assetManager.loadAsset(assetId); + } else { + auto [cached, err] = ctx.assetManager.getAsset(assetId); + if (err) { + ctx.assetManager.initTypeManager(detail::makeLoader, ctx); + OX_RETURN_ERROR(ctx.assetManager.loadAsset(assetId).moveTo(cached)); + } + return cached; + } + }; if (beginsWith(assetId, "uuid://")) { - assetId = substr(assetId, 7); + return load(ctx, substr(assetId, 7), forceLoad); } else { auto const [uuid, uuidErr] = getUuid(ctx, assetId); if (!uuidErr) { - assetId = uuid.toString(); + return load(ctx, uuid.toString(), forceLoad); + } else { + return load(ctx, assetId, forceLoad); } } - if (forceLoad) { - ctx.assetManager.initTypeManager(detail::makeLoader, ctx); - return ctx.assetManager.loadAsset(assetId); - } else { - auto [cached, err] = ctx.assetManager.getAsset(assetId); - if (err) { - ctx.assetManager.initTypeManager(detail::makeLoader, ctx); - OX_RETURN_ERROR(ctx.assetManager.loadAsset(assetId).moveTo(cached)); - } - return cached; - } } #else @@ -119,7 +126,7 @@ ox::Result> readObjNoCache( #endif -ox::Error reloadAsset(keel::Context &ctx, ox::StringView assetId) noexcept; +ox::Error reloadAsset(keel::Context &ctx, ox::StringViewCR assetId) noexcept; template ox::Result> readObj( @@ -163,7 +170,7 @@ ox::Error writeObj( ox::Error setRomFs(Context &ctx, ox::UPtr &&fs) noexcept; -ox::Result> loadRomFs(ox::StringViewCR path) noexcept; +ox::Result> loadRomFs(ox::StringViewCR path) noexcept; ox::Result loadRom(ox::StringViewCR path = "") noexcept; diff --git a/src/olympic/keel/src/media.cpp b/src/olympic/keel/src/media.cpp index 714a6fde..fdbb0111 100644 --- a/src/olympic/keel/src/media.cpp +++ b/src/olympic/keel/src/media.cpp @@ -42,7 +42,7 @@ static void clearUuidMap(Context &ctx) noexcept { ctx.pathToUuid.clear(); } -void createUuidMapping(Context &ctx, ox::StringView filePath, ox::UUID const&uuid) noexcept { +void createUuidMapping(Context &ctx, ox::StringViewCR filePath, ox::UUID const&uuid) noexcept { ctx.pathToUuid[filePath] = uuid; ctx.uuidToPath[uuid.toString()] = filePath; } @@ -88,7 +88,7 @@ ox::Result getUuid(Context &ctx, ox::FileAddress const&fileAddr) noexc return getUuid(ctx, path); } -ox::Result getUuid(Context &ctx, ox::StringView path) noexcept { +ox::Result getUuid(Context &ctx, ox::StringViewCR path) noexcept { if (beginsWith(path, "uuid://")) { auto const uuid = substr(path, 7); return ox::UUID::fromString(uuid); @@ -112,7 +112,7 @@ ox::Result getPath(Context &ctx, ox::FileAddress const&fileAddr } } -ox::Result getPath(Context &ctx, ox::CStringView fileId) noexcept { +ox::Result getPath(Context &ctx, ox::CStringViewCR fileId) noexcept { if (beginsWith(fileId, "uuid://")) { auto const uuid = substr(fileId, 7); #ifndef OX_BARE_METAL @@ -126,10 +126,9 @@ ox::Result getPath(Context &ctx, ox::CStringView fileId) noexce } } -ox::Result uuidUrlToPath(Context &ctx, ox::StringView uuid) noexcept { - uuid = substr(uuid, 7); +ox::Result uuidUrlToPath(Context &ctx, ox::StringViewCR uuid) noexcept { #ifndef OX_BARE_METAL - OX_REQUIRE_M(out, ctx.uuidToPath.at(uuid)); + OX_REQUIRE_M(out, ctx.uuidToPath.at(substr(uuid, 7))); return ox::CStringView(*out); #else return ox::Error(1, "UUID to path conversion not supported on this platform"); @@ -154,19 +153,18 @@ ox::Result uuidToPath(Context &ctx, ox::UUID const&uuid) noexce #endif } -ox::Error reloadAsset(keel::Context &ctx, ox::StringView assetId) noexcept { +ox::Error reloadAsset(keel::Context &ctx, ox::StringViewCR assetId) noexcept { ox::UUIDStr uuidStr; if (beginsWith(assetId, "uuid://")) { - assetId = substr(assetId, 7); - OX_REQUIRE(p, keel::uuidToPath(ctx, assetId)); + return ctx.assetManager.reloadAsset(substr(assetId, 7)); } else { auto const [uuid, uuidErr] = getUuid(ctx, assetId); if (!uuidErr) { - uuidStr = uuid.toString(); - assetId = uuidStr; + return ctx.assetManager.reloadAsset(uuidStr); + } else { + return ctx.assetManager.reloadAsset(assetId); } } - return ctx.assetManager.reloadAsset(assetId); } } @@ -238,7 +236,7 @@ ox::Error setRomFs(Context &ctx, ox::UPtr &&fs) noexcept { return buildUuidMap(ctx); } -ox::Result> loadRomFs(ox::StringViewCR path) noexcept { +ox::Result> loadRomFs(ox::StringViewCR path) noexcept { auto const lastDot = ox::lastIndexOf(path, '.'); if (!lastDot.error && substr(path, lastDot.value) == ".oxfs") { OX_REQUIRE(rom, loadRom(path));