sceNp/vm: Fix strings read, implement safe string reader

rpcs3/Emu/Cell/Modules/sceNp.cpp

@@ -480,7 +480,8 @@ error_code npDrmIsAvailable(vm::cptr<u8> k_licensee_addr, vm::cptr<char> drm_pat
 		sceNp.success("npDrmIsAvailable(): PSP remaster KLicense key applied.");
 	}
 
-	const std::string enc_drm_path(drm_path.get_ptr(), std::find(drm_path.get_ptr(), drm_path.get_ptr() + 0x100, '\0'));
+	std::string enc_drm_path;
+	ensure(vm::read_string(drm_path.addr(), 0x100, enc_drm_path, true), "Secret access violation");
 
 	sceNp.warning(u8"npDrmIsAvailable(): drm_path=“%s”", enc_drm_path);
 
@@ -599,8 +600,10 @@ error_code npDrmVerifyUpgradeLicense(vm::cptr<char> content_id)
 		return SCE_NP_DRM_ERROR_INVALID_PARAM;
 	}
 
-	const std::string content_str(content_id.get_ptr(), std::find(content_id.get_ptr(), content_id.get_ptr() + 0x2f, '\0'));
-	sceNp.warning("npDrmVerifyUpgradeLicense(): content_id=%s", content_id);
+	std::string content_str;
+	ensure(vm::read_string(content_id.addr(), 0x2f, content_str, true), "Secret access violation");
+
+	sceNp.warning("npDrmVerifyUpgradeLicense(): content_id=%s", content_str);
 
 	if (!rpcs3::utils::verify_c00_unlock_edat(content_str))
 		return SCE_NP_DRM_ERROR_LICENSE_NOT_FOUND;
@@ -654,7 +657,9 @@ error_code sceNpDrmGetTimelimit(vm::cptr<char> path, vm::ptr<u64> time_remain)
 		return ret;
 	}
 
-	const std::string enc_drm_path(path.get_ptr(), std::find(path.get_ptr(), path.get_ptr() + 0x100, '\0'));
+	std::string enc_drm_path;
+	ensure(vm::read_string(path.addr(), 0x100, enc_drm_path, true), "Secret access violation");
+
 	const auto [fs_error, ppath, real_path, enc_file, type] = lv2_file::open(enc_drm_path, 0, 0);
 
 	if (fs_error)

rpcs3/Emu/Cell/lv2/sys_fs.cpp

@@ -671,41 +671,29 @@ fs::file lv2_file::make_view(const std::shared_ptr<lv2_file>& _file, u64 offset)
 	return result;
 }
 
-std::pair<CellError, std::string_view> translate_to_sv(vm::cptr<char> ptr, bool is_path = true)
+std::pair<CellError, std::string> translate_to_str(vm::cptr<char> ptr, bool is_path = true)
 {
-	const u32 addr = ptr.addr();
-
-	if (!vm::check_addr(addr, vm::page_readable))
-	{
-		return {CELL_EFAULT, {}};
-	}
-
-	const usz remained_page_memory = (~addr % 4096) + 1;
-
 	constexpr usz max_length = CELL_FS_MAX_FS_PATH_LENGTH + 1;
 
-	const usz target_memory_span_size = std::min<usz>(max_length, vm::check_addr(addr + 4096, vm::page_readable) ? max_length : remained_page_memory);
-
-	std::string_view path{ptr.get_ptr(), target_memory_span_size};
-	path = path.substr(0, path.find_first_of('\0'));
+	std::string path;
 
-	if (path.size() == max_length)
+	if (!vm::read_string(ptr.addr(), max_length, path, true))
 	{
-		return {CELL_ENAMETOOLONG, {}};
+		// Null character lookup has ended whilst pointing at invalid memory
+		return {CELL_EFAULT, std::move(path)};
 	}
 
-	if (path.size() == target_memory_span_size)
+	if (path.size() == max_length)
 	{
-		// Null character lookup has ended whilst pointing at invalid memory
-		return {CELL_EFAULT, path};
+		return {CELL_ENAMETOOLONG, {}};
 	}
 
 	if (is_path && !path.starts_with("/"sv))
 	{
-		return {CELL_ENOENT, path};
+		return {CELL_ENOENT, std::move(path)};
 	}
 
-	return {{}, path};
+	return {{}, std::move(path)};
 }
 
 error_code sys_fs_test(ppu_thread&, u32 arg1, u32 arg2, vm::ptr<u32> arg3, u32 arg4, vm::ptr<char> buf, u32 buf_size)
@@ -997,7 +985,7 @@ error_code sys_fs_open(ppu_thread& ppu, vm::cptr<char> path, s32 flags, vm::ptr<
 
 	sys_fs.warning("sys_fs_open(path=%s, flags=%#o, fd=*0x%x, mode=%#o, arg=*0x%x, size=0x%llx)", path, flags, fd, mode, arg, size);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -1275,7 +1263,7 @@ error_code sys_fs_opendir(ppu_thread& ppu, vm::cptr<char> path, vm::ptr<u32> fd)
 
 	sys_fs.warning("sys_fs_opendir(path=%s, fd=*0x%x)", path, fd);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -1467,7 +1455,7 @@ error_code sys_fs_stat(ppu_thread& ppu, vm::cptr<char> path, vm::ptr<CellFsStat>
 
 	sys_fs.warning("sys_fs_stat(path=%s, sb=*0x%x)", path, sb);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -1625,7 +1613,7 @@ error_code sys_fs_mkdir(ppu_thread& ppu, vm::cptr<char> path, s32 mode)
 
 	sys_fs.warning("sys_fs_mkdir(path=%s, mode=%#o)", path, mode);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -1682,14 +1670,14 @@ error_code sys_fs_rename(ppu_thread& ppu, vm::cptr<char> from, vm::cptr<char> to
 
 	sys_fs.warning("sys_fs_rename(from=%s, to=%s)", from, to);
 
-	const auto [from_error, vfrom] = translate_to_sv(from);
+	const auto [from_error, vfrom] = translate_to_str(from);
 
 	if (from_error)
 	{
 		return {from_error, vfrom};
 	}
 
-	const auto [to_error, vto] = translate_to_sv(to);
+	const auto [to_error, vto] = translate_to_str(to);
 
 	if (to_error)
 	{
@@ -1748,7 +1736,7 @@ error_code sys_fs_rmdir(ppu_thread& ppu, vm::cptr<char> path)
 
 	sys_fs.warning("sys_fs_rmdir(path=%s)", path);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -1799,7 +1787,7 @@ error_code sys_fs_unlink(ppu_thread& ppu, vm::cptr<char> path)
 
 	sys_fs.warning("sys_fs_unlink(path=%s)", path);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -2649,7 +2637,7 @@ error_code sys_fs_get_block_size(ppu_thread& ppu, vm::cptr<char> path, vm::ptr<u
 
 	sys_fs.warning("sys_fs_get_block_size(path=%s, sector_size=*0x%x, block_size=*0x%x, arg4=*0x%x)", path, sector_size, block_size, arg4);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -2700,7 +2688,7 @@ error_code sys_fs_truncate(ppu_thread& ppu, vm::cptr<char> path, u64 size)
 
 	sys_fs.warning("sys_fs_truncate(path=%s, size=0x%llx)", path, size);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -2806,7 +2794,7 @@ error_code sys_fs_chmod(ppu_thread&, vm::cptr<char> path, s32 mode)
 {
 	sys_fs.todo("sys_fs_chmod(path=%s, mode=%#o)", path, mode);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -2959,7 +2947,7 @@ error_code sys_fs_utime(ppu_thread& ppu, vm::cptr<char> path, vm::cptr<CellFsUti
 	sys_fs.warning("sys_fs_utime(path=%s, timep=*0x%x)", path, timep);
 	sys_fs.warning("** actime=%u, modtime=%u", timep->actime, timep->modtime);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -3147,7 +3135,7 @@ error_code sys_fs_newfs(ppu_thread& ppu, vm::cptr<char> dev_name, vm::cptr<char>
 
 	sys_fs.warning("sys_fs_newfs(dev_name=%s, file_system=%s, unk1=0x%x, str1=%s)", dev_name, file_system, unk1, str1);
 
-	const auto [dev_error, device_name] = translate_to_sv(dev_name, false);
+	const auto [dev_error, device_name] = translate_to_str(dev_name, false);
 
 	if (dev_error)
 	{
@@ -3195,21 +3183,21 @@ error_code sys_fs_mount(ppu_thread& ppu, vm::cptr<char> dev_name, vm::cptr<char>
 
 	sys_fs.warning("sys_fs_mount(dev_name=%s, file_system=%s, path=%s, unk1=0x%x, prot=%d, unk3=0x%x, str1=%s, str_len=%d)", dev_name, file_system, path, unk1, prot, unk3, str1, str_len);
 
-	const auto [dev_error, device_name] = translate_to_sv(dev_name, false);
+	const auto [dev_error, device_name] = translate_to_str(dev_name, false);
 
 	if (dev_error)
 	{
 		return {dev_error, device_name};
 	}
 
-	const auto [fs_error, filesystem] = translate_to_sv(file_system, false);
+	const auto [fs_error, filesystem] = translate_to_str(file_system, false);
 
 	if (fs_error)
 	{
 		return {fs_error, filesystem};
 	}
 
-	const auto [path_error, path_sv] = translate_to_sv(path);
+	const auto [path_error, path_sv] = translate_to_str(path);
 
 	if (path_error)
 	{
@@ -3298,7 +3286,7 @@ error_code sys_fs_unmount(ppu_thread& ppu, vm::cptr<char> path, s32 unk1, s32 un
 
 	sys_fs.warning("sys_fs_unmount(path=%s, unk1=0x%x, unk2=0x%x)", path, unk1, unk2);
 
-	const auto [path_error, vpath] = translate_to_sv(path);
+	const auto [path_error, vpath] = translate_to_str(path);
 
 	if (path_error)
 	{

rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp

@@ -524,7 +524,12 @@ error_code _sys_ppu_thread_create(ppu_thread& ppu, vm::ptr<u64> thread_id, vm::p
 	{
 		constexpr u32 max_size = c_max_ppu_name_size; // max size including null terminator
 		const auto pname = threadname.get_ptr();
-		ppu_name.assign(pname, std::find(pname, pname + max_size, '\0'));
+
+		if (!vm::read_string(threadname.addr(), max_size, ppu_name, true))
+		{
+			dct.free(stack_size);
+			return CELL_EFAULT;
+		}
 	}
 
 	const u32 tid = idm::import<named_thread<ppu_thread>>([&]()
@@ -615,10 +620,15 @@ error_code sys_ppu_thread_rename(ppu_thread& ppu, u32 thread_id, vm::cptr<char>
 	}
 
 	constexpr u32 max_size = c_max_ppu_name_size; // max size including null terminator
-	const auto pname = name.get_ptr();
 
 	// Make valid name
-	auto _name = make_single<std::string>(pname, std::find(pname, pname + max_size, '\0'));
+	std::string out_str;
+	if (!vm::read_string(name.addr(), max_size, out_str, true))
+	{
+		return CELL_EFAULT;
+	}
+
+	auto _name = make_single<std::string>(std::move(out_str));
 
 	// thread_ctrl name is not changed (TODO)
 	sys_ppu_thread.warning(u8"sys_ppu_thread_rename(): Thread renamed to “%s”", *_name);

rpcs3/Emu/Memory/vm.cpp

@@ -2323,6 +2323,54 @@ namespace vm
 
 		return 0;
 	}
+
+	bool read_string(u32 addr, u32 max_size, std::string& out_string, bool check_pages) noexcept
+	{
+		if (!max_size)
+		{
+			return true;
+		}
+
+		// Prevent overflow
+		const u32 size = 0 - max_size < addr ? (0 - addr) : max_size;
+
+		for (u32 i = addr, end = utils::align(addr + size, 4096) - 1; i <= end;)
+		{
+			if (check_pages && !vm::check_addr(i, vm::page_readable))
+			{
+				// Invalid string termination
+				return false;
+			}
+
+			const char* s_start = vm::get_super_ptr<const char>(i);
+			const u32 space = std::min<u32>(end - i + 1, 4096 - (i % 4096));
+			const char* s_end = s_start + space;
+			const char* s_null = std::find(s_start, s_end, '\0');
+
+			// Append string
+			out_string.append(s_start, s_null);
+
+			// Recheck for zeroes after append
+			const usz old_size = out_string.size();
+			out_string.erase(std::find(out_string.end() - (s_null - s_start), out_string.end(), '\0'), out_string.end());
+
+			if (out_string.size() != old_size || s_null != s_end)
+			{
+				// Null terminated
+				return true;
+			}
+
+			i += space;
+
+			if (!i)
+			{
+				break;
+			}
+		}
+
+		// Non-null terminated but terminated by size limit (so the string may continue)
+		return size == max_size;
+	}
 }
 
 void fmt_class_string<vm::_ptr_base<const void, u32>>::format(std::string& out, u64 arg)
@@ -2339,8 +2387,8 @@ void fmt_class_string<vm::_ptr_base<const char, u32>>::format(std::string& out,
 		return;
 	}
 
-	// Filter certainly invalid addresses (TODO)
-	if (arg < 0x10000 || arg >= 0xf0000000)
+	// Filter certainly invalid addresses
+	if (!vm::check_addr(arg, vm::page_readable))
 	{
 		out += reinterpret_cast<const char*>(u8"«INVALID_ADDRESS:");
 		fmt_class_string<u32>::format(out, arg);
@@ -2352,26 +2400,14 @@ void fmt_class_string<vm::_ptr_base<const char, u32>>::format(std::string& out,
 
 	out += reinterpret_cast<const char*>(u8"“");
 
-	for (vm::_ptr_base<const volatile char, u32> ptr = vm::cast(arg);; ptr++)
+	if (!vm::read_string(arg, umax, out, true))
 	{
-		if (!vm::check_addr(ptr.addr()))
-		{
-			// TODO: optimize checks
-			out.resize(start);
-			out += reinterpret_cast<const char*>(u8"«INVALID_ADDRESS:");
-			fmt_class_string<u32>::format(out, arg);
-			out += reinterpret_cast<const char*>(u8"»");
-			return;
-		}
-
-		if (const char ch = *ptr)
-		{
-			out += ch;
-		}
-		else
-		{
-			break;
-		}
+		// Revert changes
+		out.resize(start);
+		out += reinterpret_cast<const char*>(u8"«INVALID_ADDRESS:");
+		fmt_class_string<u32>::format(out, arg);
+		out += reinterpret_cast<const char*>(u8"»");
+		return;
 	}
 
 	out += reinterpret_cast<const char*>(u8"”");

rpcs3/Emu/Memory/vm.h

@@ -80,6 +80,9 @@ namespace vm
 		return !(~g_pages[addr / 4096] & (flags | page_allocated));
 	}
 
+	// Read string in a safe manner (page aware) (bool true = if null-termination)
+	bool read_string(u32 addr, u32 max_size, std::string& out_string, bool check_pages = true) noexcept;
+
 	// Search and map memory in specified memory location (min alignment is 0x10000)
 	u32 alloc(u32 size, memory_location_t location, u32 align = 0x10000);
 

rpcs3/util/asm.hpp

@@ -375,10 +375,10 @@ namespace utils
 	}
 
 	// Align to power of 2
-	template <typename T, typename = std::enable_if_t<std::is_unsigned_v<T>>>
-	constexpr T align(T value, std::type_identity_t<T> align)
+	template <typename T, typename U, typename = std::enable_if_t<std::is_unsigned_v<T>>>
+	constexpr std::make_unsigned_t<std::common_type_t<T, U>> align(T value, U align)
 	{
-		return static_cast<T>((value + (align - 1)) & (T{0} - align));
+		return static_cast<std::make_unsigned_t<std::common_type_t<T, U>>>((value + (align - 1)) & (T{0} - align));
 	}
 
 	// General purpose aligned division, the result is rounded up not truncated