From c71fa5e32b545216a6c5276ae3da85b7d30d4eb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 8 Sep 2026 18:06:55 -0600 Subject: [PATCH] sceIo: stop clobbering st_private, report FAT permissions, implement sceIoChstat Cashing in the io/stat and io/shortname recordings. __IoGetStat began with memset(stat, 0xfe, sizeof(SceIoStat)), which destroyed 24 bytes of the caller's buffer that a real PSP never touches - it writes only as far as the timestamps and leaves all six st_private words exactly as it found them. It also wrote a made-up sector number into st_private[0] on the memory stick. That word carries the LBN on a UMD, which games read to build disc0:/sce_lbn paths, so it stays for non-FAT and is left alone otherwise. FAT has no permissions of its own and everything reads back as 0777. We were passing the host's idea of the file through instead. The existing "all files look executable on FAT" hack for Beats (issue #14812) was right in substance but lived only in sceIoDread, so sceIoGetstat and sceIoDread disagreed about the same file where hardware has them agree. Both now go through one path, which also gets the read-only case right: no write bits means mode 0555 and attr 0x21. sceIoGetstat on the root of a volume is refused, as on hardware. sceIoChstat was a logging stub. It now applies the read-only flag, which is what st_mode's write bits and st_attr's 0x01 both mean on FAT - setting either produces both, and it's reversible. That needs a new IFileSystem::SetFileWritable, defaulting to "can't" so read-only filesystems and hosts that can't express it (Android content URIs) are unaffected; the call still succeeds there, since hardware would have. GenerateFatShortNames now accounts for capitalisation. FAT keeps a lowercase flag for the base and another for the extension, but the PSP only honours the base one, so "shrt" becomes SHRT while "readme.txt" becomes README~1.TXT. We were only adding a counter on collision. The unit test carries the whole recorded set, including the corrected README~1.MD. io/shortname stays in tests_next: its d_name column can't match while SimulateVFATBug is uppercasing lowercase 8.3 names, which is deliberate and load-bearing for homebrew. --- Common/File/FileUtil.cpp | 35 +++++++++++ Common/File/FileUtil.h | 4 ++ Core/FileSystems/DirectoryFileSystem.cpp | 5 ++ Core/FileSystems/DirectoryFileSystem.h | 1 + Core/FileSystems/FileSystem.cpp | 28 +++++++++ Core/FileSystems/FileSystem.h | 3 + Core/FileSystems/MetaFileSystem.cpp | 13 ++++ Core/FileSystems/MetaFileSystem.h | 1 + Core/HLE/sceIo.cpp | 76 ++++++++++++++++++------ pspautotests | 2 +- test.py | 3 +- unittest/UnitTest.cpp | 33 +++++++++- 12 files changed, 183 insertions(+), 21 deletions(-) diff --git a/Common/File/FileUtil.cpp b/Common/File/FileUtil.cpp index 218669cd68..46b92dac24 100644 --- a/Common/File/FileUtil.cpp +++ b/Common/File/FileUtil.cpp @@ -1080,6 +1080,41 @@ bool IsDirectoryWritable(const Path &path) { return true; } +bool SetFileWritable(const Path &filename, bool writable) { + switch (filename.Type()) { + case PathType::NATIVE: + break; + default: + // Content URIs and the other virtual path types have no notion of this, so say so rather + // than pretending it worked. + return false; + } + +#ifdef _WIN32 + const DWORD attrs = GetFileAttributesW(filename.ToWString().c_str()); + if (attrs == INVALID_FILE_ATTRIBUTES) { + return false; + } + const DWORD updated = writable ? (attrs & ~FILE_ATTRIBUTE_READONLY) : (attrs | FILE_ATTRIBUTE_READONLY); + if (updated == attrs) { + return true; + } + return SetFileAttributesW(filename.ToWString().c_str(), updated) != 0; +#else + struct stat info; + if (stat(filename.c_str(), &info) != 0) { + return false; + } + // Mirror the write bits onto whoever already has read access, which is what clearing the FAT + // read-only attribute amounts to. + mode_t mode = info.st_mode & ~(mode_t)0222; + if (writable) { + mode |= (info.st_mode & 0444) >> 1; + } + return chmod(filename.c_str(), mode) == 0; +#endif +} + // Deletes an empty directory, returns true on success // WARNING: On Android with content URIs, it will delete recursively! bool DeleteDir(const Path &path) { diff --git a/Common/File/FileUtil.h b/Common/File/FileUtil.h index 9b5a0e12cf..23573c1911 100644 --- a/Common/File/FileUtil.h +++ b/Common/File/FileUtil.h @@ -127,6 +127,10 @@ bool CreateEmptyFile(const Path &filename); // don't tell the whole story (Windows ACLs, read-only mounts, ...). bool IsDirectoryWritable(const Path &path); +// Set or clear a file's read-only-ness, which is what a FAT read-only attribute maps onto. +// Returns false where the platform can't express it - notably Android content URIs. +bool SetFileWritable(const Path &filename, bool writable); + // Opens ini file (cheats, texture replacements etc.) // TODO: Belongs in System or something. bool OpenFileInEditor(const Path &fileName); diff --git a/Core/FileSystems/DirectoryFileSystem.cpp b/Core/FileSystems/DirectoryFileSystem.cpp index 6bc1fa57e9..2a8a0c2f50 100644 --- a/Core/FileSystems/DirectoryFileSystem.cpp +++ b/Core/FileSystems/DirectoryFileSystem.cpp @@ -634,6 +634,11 @@ bool DirectoryFileSystem::RemoveFile(const std::string &filename) { return ReplayApplyDisk(ReplayAction::FILE_REMOVE, retValue, CoreTiming::GetGlobalTimeUs()) != 0; } +bool DirectoryFileSystem::SetFileWritable(const std::string &filename, bool writable) { + Path fullName = GetLocalPath(filename); + return File::SetFileWritable(fullName, writable); +} + // Note that this runs *before* the literal path is tried, not as a fallback after it fails. That's // deliberate: on Windows the host resolves its own 8.3 aliases, which are generated by a different // rule than ours, so opening the literal name can quietly land on a different file than the one we diff --git a/Core/FileSystems/DirectoryFileSystem.h b/Core/FileSystems/DirectoryFileSystem.h index 7d0a276397..70d0411d1c 100644 --- a/Core/FileSystems/DirectoryFileSystem.h +++ b/Core/FileSystems/DirectoryFileSystem.h @@ -85,6 +85,7 @@ public: bool RmDir(const std::string &dirname) override; int RenameFile(const std::string &from, const std::string &to) override; bool RemoveFile(const std::string &filename) override; + bool SetFileWritable(const std::string &filename, bool writable) override; FileSystemFlags Flags() const override { return flags; } u64 FreeDiskSpace(const std::string &path) override; diff --git a/Core/FileSystems/FileSystem.cpp b/Core/FileSystems/FileSystem.cpp index c45ac269a1..cf24c002a5 100644 --- a/Core/FileSystems/FileSystem.cpp +++ b/Core/FileSystems/FileSystem.cpp @@ -103,6 +103,30 @@ static std::string CleanShortNamePart(std::string_view part, size_t maxLen, bool return out; } +// Whether the name's capitalisation survives without a long-name entry. FAT keeps one flag for +// the base and one for the extension, but the PSP only honours the base one - so a lowercase +// extension forces a long name entry, and with it a ~1 suffix, while a lowercase base alone +// doesn't. That's why hardware gives "shrt" -> SHRT but "readme.txt" -> README~1.TXT. +static bool ShortNameCaseSurvives(std::string_view base, std::string_view ext) { + bool lower = false, upper = false; + for (char c : base) { + if (c >= 'a' && c <= 'z') { + lower = true; + } else if (c >= 'A' && c <= 'Z') { + upper = true; + } + } + if (lower && upper) { + return false; + } + for (char c : ext) { + if (c >= 'a' && c <= 'z') { + return false; + } + } + return true; +} + void GenerateFatShortNames(const std::vector &listing, std::vector *shortNames) { shortNames->clear(); shortNames->reserve(listing.size()); @@ -128,6 +152,10 @@ void GenerateFatShortNames(const std::vector &listing, std::vector< lossy = true; } + if (!ShortNameCaseSurvives(baseIn, extIn)) { + lossy = true; + } + std::string base = CleanShortNamePart(baseIn, 8, &lossy); std::string ext = CleanShortNamePart(extIn, 3, &lossy); if (base.empty()) { diff --git a/Core/FileSystems/FileSystem.h b/Core/FileSystems/FileSystem.h index 4e59d3811a..2dc25f9afb 100644 --- a/Core/FileSystems/FileSystem.h +++ b/Core/FileSystems/FileSystem.h @@ -157,6 +157,9 @@ public: virtual bool RmDir(const std::string &dirname) = 0; virtual int RenameFile(const std::string &from, const std::string &to) = 0; virtual bool RemoveFile(const std::string &filename) = 0; + // Sets or clears the FAT read-only attribute, as sceIoChstat does. Defaults to "can't", which + // is right for read-only filesystems and for hosts that can't express it. + virtual bool SetFileWritable(const std::string &filename, bool writable) { return false; } virtual int Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outdataPtr, u32 outlen, int &usec) = 0; virtual PSPDevType DevType(u32 handle) = 0; virtual FileSystemFlags Flags() const = 0; diff --git a/Core/FileSystems/MetaFileSystem.cpp b/Core/FileSystems/MetaFileSystem.cpp index f87cddba9a..d1065b17c3 100644 --- a/Core/FileSystems/MetaFileSystem.cpp +++ b/Core/FileSystems/MetaFileSystem.cpp @@ -507,6 +507,19 @@ int MetaFileSystem::RenameFile(const std::string &from, const std::string &to) } } +bool MetaFileSystem::SetFileWritable(const std::string &filename, bool writable) +{ + std::lock_guard guard(lock); + std::string of; + IFileSystem *system; + int error = MapFilePath(filename, &of, &system); + if (error == 0) { + return system->SetFileWritable(of, writable); + } else { + return false; + } +} + bool MetaFileSystem::RemoveFile(const std::string &filename) { std::lock_guard guard(lock); diff --git a/Core/FileSystems/MetaFileSystem.h b/Core/FileSystems/MetaFileSystem.h index dc71263b99..ffbde32dd1 100644 --- a/Core/FileSystems/MetaFileSystem.h +++ b/Core/FileSystems/MetaFileSystem.h @@ -137,6 +137,7 @@ public: bool RmDir(const std::string &dirname) override; int RenameFile(const std::string &from, const std::string &to) override; bool RemoveFile(const std::string &filename) override; + bool SetFileWritable(const std::string &filename, bool writable) override; int Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outdataPtr, u32 outlen, int &usec) override; PSPDevType DevType(u32 handle) override; FileSystemFlags Flags() const override { return FileSystemFlags::NONE; } diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index 5b3d33fb5e..2418c5bca2 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -871,9 +871,12 @@ void ConvertTmToPspDateTime(ScePspDateTime& date_out, const tm& date_in, int mic date_out.microsecond = microSeconds; } -static void __IoGetStat(SceIoStat *stat, const PSPFileInfo &info) { - memset(stat, 0xfe, sizeof(SceIoStat)); - +// isFAT is whether the file lives on a FAT volume (the memory stick), which changes both the +// permissions reported and whether st_private means anything. +static void __IoGetStat(SceIoStat *stat, const PSPFileInfo &info, bool isFAT) { + // Deliberately no memset: pspautotests io/stat poisons the struct and shows a real PSP writes + // only as far as the timestamps - the six st_private words come back exactly as the caller + // left them. Clearing the whole struct would destroy 24 bytes the kernel never touches. int type, attr; if (info.type & FILETYPE_DIRECTORY) { type = SCE_STM_FDIR; @@ -883,13 +886,26 @@ static void __IoGetStat(SceIoStat *stat, const PSPFileInfo &info) { attr = TYPE_FILE; } - stat->st_mode = type | info.access; - stat->st_attr = attr; + if (isFAT) { + // FAT has no permissions of its own, so everything reads back as 0777 - including the + // execute bits, which is what Beats needed (issue #14812). Clearing the write bits is + // the read-only attribute, and that shows up in st_attr too. + const bool readOnly = (info.access & 0222) == 0; + stat->st_mode = type | (readOnly ? 0555 : 0777); + stat->st_attr = attr | (readOnly ? 0x01 : 0x00); + } else { + stat->st_mode = type | info.access; + stat->st_attr = attr; + } stat->st_size = info.size; ConvertTmToPspDateTime(stat->st_a_time, info.atime, info.atimeUs); ConvertTmToPspDateTime(stat->st_c_time, info.ctime, info.ctimeUs); ConvertTmToPspDateTime(stat->st_m_time, info.mtime, info.mtimeUs); - stat->st_private[0] = info.startSector; + // st_private[0] carries the LBN on a UMD, which games read to build disc0:/sce_lbn paths - + // see umd/raw_access. On the memory stick a real PSP leaves it alone entirely. + if (!isFAT) { + stat->st_private[0] = info.startSector; + } } static void __IoSchedAsync(FileNode *f, int fd, int usec) { @@ -911,11 +927,25 @@ static u32 sceIoGetstat(const char *filename, u32 addr) { // TODO: Improve timing (although this seems normally slow..) int usec = 1000; + // A real PSP refuses to stat the root of a volume - io/stat records sceIoGetstat("ms0:/") + // coming back as an invalid argument rather than describing the directory. + const char *colon = strchr(filename, ':'); + if (colon != nullptr) { + const char *rest = colon + 1; + while (*rest == '/') { + ++rest; + } + if (*rest == '\0') { + return hleDelayResult(hleLogWarning(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "volume root"), "io getstat", usec); + } + } + + const bool isFAT = pspFileSystem.FlagsFromFilename(filename) & FileSystemFlags::SIMULATE_FAT32; auto stat = PSPPointer::Create(addr); PSPFileInfo info = pspFileSystem.GetFileInfo(filename); if (info.exists) { if (stat.IsValid()) { - __IoGetStat(stat, info); + __IoGetStat(stat, info, isFAT); stat.NotifyWrite("IoGetstat"); return hleDelayResult(hleLogDebug(Log::sceIo, 0, "sector = %08x", info.startSector), "io getstat", usec); } else { @@ -931,12 +961,26 @@ static u32 sceIoChstat(const char *filename, u32 iostatptr, u32 changebits) { if (!iostat.IsValid()) return hleReportError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "bad address"); - ERROR_LOG(Log::sceIo, "UNIMPL sceIoChstat(%s, %08x, %08x)", filename, iostatptr, changebits); - if (changebits & SCE_CST_MODE) - ERROR_LOG_REPORT(Log::sceIo, "sceIoChstat: change mode to %03o requested", iostat->st_mode); + // On a FAT volume the write bits in st_mode and the 0x01 bit in st_attr are two views of the + // same read-only flag: io/stat/readonly records that setting either one produces both, and + // that it's reversible. Anything else in the struct is still ignored. + bool haveWritable = false; + bool writable = false; + if (changebits & SCE_CST_MODE) { + writable = (iostat->st_mode & 0222) != 0; + haveWritable = true; + } if (changebits & SCE_CST_ATTR) { - // These are pretty much all of the reported calls: https://report.ppsspp.org/logs/kind/1115 - ERROR_LOG_REPORT(Log::sceIo, "sceIoChstat: change attr to %04x requested", iostat->st_attr); + // The attribute wins if both were asked for, since it names the flag directly. + writable = (iostat->st_attr & 0x01) == 0; + haveWritable = true; + } + if (haveWritable) { + if (!pspFileSystem.SetFileWritable(filename, writable)) { + // Nothing to be done on a host that can't express it - Android content URIs, or a + // read-only filesystem. Hardware would have succeeded, so don't fail the call. + WARN_LOG(Log::sceIo, "sceIoChstat: could not make %s %s", filename, writable ? "writable" : "read-only"); + } } if (changebits & SCE_CST_SIZE) ERROR_LOG(Log::sceIo, "sceIoChstat: change size requested"); @@ -2533,17 +2577,15 @@ static u32 sceIoDread(int id, u32 dirent_addr) { } PSPFileInfo &info = dir->listing[dir->index]; - __IoGetStat(&entry->d_stat, info); + const bool isFATDir = pspFileSystem.FlagsFromFilename(dir->name) & FileSystemFlags::SIMULATE_FAT32; + __IoGetStat(&entry->d_stat, info, isFATDir); strncpy(entry->d_name, info.name.c_str(), 256); entry->d_name[255] = '\0'; - bool isFAT = pspFileSystem.FlagsFromFilename(dir->name) & FileSystemFlags::SIMULATE_FAT32; // Only write d_private for memory stick - if (isFAT) { + if (isFATDir) { const std::string &shortName = dir->ShortName(dir->index); - // All files look like they're executable on FAT. This is required for Beats, see issue #14812 - entry->d_stat.st_mode |= 0111; // write d_private for supporting Custom BGM // ref JPCSP https://code.google.com/p/jpcsp/source/detail?r=3468 if (Memory::IsValidAddress(entry->d_private)){ diff --git a/pspautotests b/pspautotests index dcc31bc21c..f25996c927 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit dcc31bc21cab554c8cff226898a93c84e7b90ae3 +Subproject commit f25996c927efacd225b590fee4d53a5efeceee4b diff --git a/test.py b/test.py index e66c040a90..22142cb689 100755 --- a/test.py +++ b/test.py @@ -221,6 +221,8 @@ tests_good = [ "io/cwd/cwd", "io/file/rename", "io/directory/directory", + "io/stat/stat", + "io/stat/readonly", "io/open/badparent", "jpeg/create", "jpeg/delete", @@ -466,7 +468,6 @@ tests_next = [ "io/io/io", "io/iodrv/iodrv", "io/shortname/shortname", - "io/stat/stat", "io/open/tty0", "jpeg/csc", "jpeg/decode", diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 9086c598f1..fc4657c9f7 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -2914,14 +2914,29 @@ bool TestFatShortNames() { return shortNames; }; - // Names that already fit 8.3 are only uppercased, and the navigation entries are left alone. + // A name that is already valid uppercase 8.3 is kept as-is, and the navigation entries are + // left alone. "readme.md" is not: its extension is lowercase, which a PSP can't record, so it + // gets a counter - see the case block below. std::vector plain = shortNamesFor({".", "..", "TEST.TXT", "readme.md", "WIPEOUT"}); EXPECT_EQ_STR(plain[0], std::string(".")); EXPECT_EQ_STR(plain[1], std::string("..")); EXPECT_EQ_STR(plain[2], std::string("TEST.TXT")); - EXPECT_EQ_STR(plain[3], std::string("README.MD")); + EXPECT_EQ_STR(plain[3], std::string("README~1.MD")); EXPECT_EQ_STR(plain[4], std::string("WIPEOUT")); + // Capitalisation, as recorded off a real PSP by pspautotests io/shortname. FAT keeps a + // lowercase flag for the base and another for the extension, but the PSP only honours the + // base one - so a lowercase base survives on its own and a lowercase extension never does. + std::vector cased = shortNamesFor({"shrt", "readme.txt", "UPPER.TXT", "MiXeD.txt"}); + // All lowercase, no extension: representable, so no counter. + EXPECT_EQ_STR(cased[0], std::string("SHRT")); + // Lowercase extension: not representable. + EXPECT_EQ_STR(cased[1], std::string("README~1.TXT")); + // Already uppercase throughout. + EXPECT_EQ_STR(cased[2], std::string("UPPER.TXT")); + // Mixed case in the base. + EXPECT_EQ_STR(cased[3], std::string("MIXED~1.TXT")); + // Long names get truncated to six characters plus a counter, which keeps counting past ~4. std::vector many = shortNamesFor({ "sample-12s.mp3", @@ -2945,9 +2960,23 @@ bool TestFatShortNames() { std::vector odd = shortNamesFor({"my song.mp3", "a+b.mp3", "no_ext", ".hidden"}); EXPECT_EQ_STR(odd[0], std::string("MYSONG~1.MP3")); EXPECT_EQ_STR(odd[1], std::string("A_B~1.MP3")); + // All lowercase with no extension, so this one keeps its name. EXPECT_EQ_STR(odd[2], std::string("NO_EXT")); EXPECT_EQ_STR(odd[3], std::string("HIDDEN~1")); + // The rest of what io/shortname records, so the whole recorded set is pinned here and not + // only in a test that needs a PSP to re-run. + std::vector hw = shortNamesFor({ + "a.b.c.txt", "noextensionhere", "sp ace.txt", "+plus[brack].txt", + "toolongextension.mpeg", "LongDirectoryName", + }); + EXPECT_EQ_STR(hw[0], std::string("ABC~1.TXT")); + EXPECT_EQ_STR(hw[1], std::string("NOEXTE~1")); + EXPECT_EQ_STR(hw[2], std::string("SPACE~1.TXT")); + EXPECT_EQ_STR(hw[3], std::string("_PLUS_~1.TXT")); + EXPECT_EQ_STR(hw[4], std::string("TOOLON~1.MPE")); + EXPECT_EQ_STR(hw[5], std::string("LONGDI~1")); + // Two long names sharing a six character stem must not collide. std::vector collide = shortNamesFor({"longname-one.txt", "longname-two.txt"}); EXPECT_EQ_STR(collide[0], std::string("LONGNA~1.TXT"));