Firmware installation bugfixes (#9855)

* Fix race condition in PUP installation abortion.
* Fix freezes of emulator in case the PUP installation failed due to filesystem errors.
* Use fs::create_path as opposed to fs::create_dir as it is can create upper directories in case they are missing and is better in error handling.
* Report TAR errors on failure to create directories.
* Fix pup_object constructor to not crash on invalid PUP file header. (report an error)
* Fix pup_object::validate_hashes to not crash on invalid PUP file entries. (report an error)
* Do not call Qt functions inside a named_thread because it is wrong.

rpcs3/Loader/PUP.cpp

@@ -9,23 +9,37 @@ pup_object::pup_object(const fs::file& file): m_file(file)
 {
 	if (!file)
 	{
-		isValid = false;
 		return;
 	}
 
 	m_file.seek(0);
-	PUPHeader m_header;
+	PUPHeader m_header{};
-	m_file.read(m_header);
+
-	if (m_header.magic != "SCEUF\0\0\0"_u64)
+	if (!m_file.read(m_header) || m_header.magic != "SCEUF\0\0\0"_u64)
 	{
-		isValid = false;
+		// Either file is not large enough to contain header or magic is invalid
+		return;
+	}
+
+	constexpr usz entry_size = sizeof(PUPFileEntry) + sizeof(PUPHashEntry);
+
+	if (!m_header.file_count || (m_file.size() - sizeof(PUPHeader)) / entry_size < m_header.file_count)
+	{
+		// These checks before read() are to avoid some std::bad_alloc exceptions when file_count is too large
+		// So we cannot rely on read() for error checking in such cases
 		return;
 	}
 
 	m_file_tbl.resize(m_header.file_count);
-	m_file.read(m_file_tbl);
 	m_hash_tbl.resize(m_header.file_count);
-	m_file.read(m_hash_tbl);
+
+	if (!m_file.read(m_file_tbl) || !m_file.read(m_hash_tbl))
+	{
+		// If these fail it is an unexpected filesystem error, because file size must suffice as we checked in previous checks
+		return;
+	}
+
+	isValid = true;
 }
 
 fs::file pup_object::get_file(u64 entry_id)
@@ -47,11 +61,19 @@ fs::file pup_object::get_file(u64 entry_id)
 
 bool pup_object::validate_hashes()
 {
+	if (!isValid) return false;
+
 	for (usz i = 0; i < m_file_tbl.size(); i++)
 	{
 		u8 *hash = m_hash_tbl[i].hash;
 		PUPFileEntry file = m_file_tbl[i];
 
+		// Sanity check for offset and length, use substraction to avoid overflows
+		if (usz size = m_file.size(); size < file.data_offset || size - file.data_offset < file.data_length)
+		{
+			return false;
+		}
+
 		std::vector<u8> buffer(file.data_length);
 		m_file.seek(file.data_offset);
 		m_file.read(buffer.data(), file.data_length);

rpcs3/Loader/PUP.h

@@ -33,7 +33,7 @@ struct PUPHashEntry
 class pup_object
 {
 	const fs::file& m_file;
-	bool isValid = true;
+	bool isValid = false;
 
 	std::vector<PUPFileEntry> m_file_tbl;
 	std::vector<PUPHashEntry> m_hash_tbl;

rpcs3/Loader/TAR.cpp

@@ -131,7 +131,12 @@ bool tar_object::extract(std::string path, std::string ignore)
 
 		case '5':
 		{
-			fs::create_dir(result);
+			if (!fs::create_path(result))
+			{
+				tar_log.error("TAR Loader: failed to create directory %s (%s)", header.name, fs::g_tls_error);
+				return false;
+			}
+
 			break;
 		}
 

rpcs3/rpcs3qt/main_window.cpp

@@ -792,18 +792,26 @@ void main_window::HandlePupInstallation(QString file_path)
 	m_gui_settings->SetValue(gui::fd_install_pup, QFileInfo(file_path).path());
 	const std::string path = sstr(file_path);
 
+	auto critical = [this](QString str)
+	{
+		Emu.CallAfter([this, str = std::move(str)]()
+		{
+			QMessageBox::critical(this, tr("Firmware Installation Failed"), str);
+		}, false);
+	};
+
 	fs::file pup_f(path);
 	if (!pup_f)
 	{
 		gui_log.error("Error opening PUP file %s", path);
-		QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: The selected firmware file couldn't be opened."));
+		critical(tr("Firmware installation failed: The selected firmware file couldn't be opened."));
 		return;
 	}
 
 	if (pup_f.size() < sizeof(PUPHeader))
 	{
 		gui_log.error("Too small PUP file: %llu", pup_f.size());
-		QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: The provided file is empty."));
+		critical(tr("Firmware installation failed: The provided file is empty."));
 		return;
 	}
 
@@ -814,7 +822,7 @@ void main_window::HandlePupInstallation(QString file_path)
 	if (header.header_length + header.data_length != pup_f.size())
 	{
 		gui_log.error("Firmware size mismatch, expected: %llu + %llu, actual: %llu", header.header_length, header.data_length, pup_f.size());
-		QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: The provided file is incomplete. Try redownloading it."));
+		critical(tr("Firmware installation failed: The provided file is incomplete. Try redownloading it."));
 		return;
 	}
 
@@ -822,14 +830,14 @@ void main_window::HandlePupInstallation(QString file_path)
 	if (!pup)
 	{
 		gui_log.error("Error while installing firmware: PUP file is invalid.");
-		QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: The provided file is corrupted."));
+		critical(tr("Firmware installation failed: The provided file is corrupted."));
 		return;
 	}
 
 	if (!pup.validate_hashes())
 	{
 		gui_log.error("Error while installing firmware: Hash check failed.");
-		QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: The provided file's contents are corrupted."));
+		critical(tr("Firmware installation failed: The provided file's contents are corrupted."));
 		return;
 	}
 
@@ -864,15 +872,13 @@ void main_window::HandlePupInstallation(QString file_path)
 	pdlg.show();
 
 	// Synchronization variable
-	atomic_t<int> progress(0);
+	atomic_t<uint> progress(0);
 	{
 		// Run asynchronously
 		named_thread worker("Firmware Installer", [&]
 		{
 			for (const auto& update_filename : update_filenames)
 			{
-				if (progress == -1) break;
-
 				fs::file update_file = update_files.get_file(update_filename);
 
 				SCEDecrypter self_dec(update_file);
@@ -884,50 +890,59 @@ void main_window::HandlePupInstallation(QString file_path)
 				if (dev_flash_tar_f.size() < 3)
 				{
 					gui_log.error("Error while installing firmware: PUP contents are invalid.");
-					QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: Firmware could not be decompressed"));
+					critical(tr("Firmware installation failed: Firmware could not be decompressed"));
 					progress = -1;
+					return;
 				}
 
 				tar_object dev_flash_tar(dev_flash_tar_f[2]);
 				if (!dev_flash_tar.extract(g_cfg.vfs.get_dev_flash(), "dev_flash/"))
 				{
 					gui_log.error("Error while installing firmware: TAR contents are invalid.");
-					QMessageBox::critical(this, tr("Firmware Installation Failed"), tr("Firmware installation failed: Firmware contents could not be extracted."));
+					critical(tr("Firmware installation failed: Firmware contents could not be extracted."));
 					progress = -1;
+					return;
 				}
 
-				if (progress >= 0)
+				if (!progress.try_inc(::narrow<uint>(update_filenames.size())))
-					progress += 1;
+				{
+					// Installation was cancelled
+					return;
+				}
 			}
 		});
 
 		// Wait for the completion
-		while (std::this_thread::sleep_for(5ms), std::abs(progress) < pdlg.maximum())
+		for (uint value = progress.load(); value < update_filenames.size(); std::this_thread::sleep_for(5ms), value = progress)
 		{
 			if (pdlg.wasCanceled())
 			{
 				progress = -1;
 				break;
 			}
+
 			// Update progress window
-			pdlg.SetValue(static_cast<int>(progress));
+			pdlg.SetValue(static_cast<int>(value));
 			QCoreApplication::processEvents();
 		}
 
-		update_files_f.close();
+		// Join thread
-		pup_f.close();
+		worker();
+	}
 
-		if (progress > 0)
+	update_files_f.close();
-		{
+	pup_f.close();
-			pdlg.SetValue(pdlg.maximum());
+
-			std::this_thread::sleep_for(100ms);
+	if (progress == update_filenames.size())
-		}
+	{
+		pdlg.SetValue(pdlg.maximum());
+		std::this_thread::sleep_for(100ms);
 	}
 
 	// Update with newly installed PS3 fonts
 	Q_EMIT RequestGlobalStylesheetChange();
 
-	if (progress > 0)
+	if (progress == update_filenames.size())
 	{
 		gui_log.success("Successfully installed PS3 firmware version %s.", version_string);
 		m_gui_settings->ShowInfoBox(tr("Success!"), tr("Successfully installed PS3 firmware and LLE Modules!"), gui::ib_pup_success, this);