From 2d473c05b004d19418b3deaaefcc8e72e0733060 Mon Sep 17 00:00:00 2001
From: Jonathan G Rennison <j.g.rennison@gmail.com>
Date: Tue, 7 Mar 2017 19:12:15 +0000
Subject: [PATCH] Fix use-after-free when switching blitters in SDL and Win32
 modes.

This mostly occurs when disabling pallete animation when fast-forward
is enabled.
---
 src/gfxinit.cpp            |  6 ++++++
 src/video/sdl_v.cpp        | 11 +++++++++--
 src/video/sdl_v.h          |  4 ++++
 src/video/video_driver.hpp | 13 +++++++++++++
 src/video/win32_v.cpp      | 11 +++++++++--
 src/video/win32_v.h        |  4 ++++
 6 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/gfxinit.cpp b/src/gfxinit.cpp
index 0acf695781..6fb6f9dce1 100644
--- a/src/gfxinit.cpp
+++ b/src/gfxinit.cpp
@@ -23,6 +23,7 @@
 #include "clear_map.h"
 #include "clear_func.h"
 #include "tree_map.h"
+#include "scope.h"
 #include "table/tree_land.h"
 #include "blitter/32bpp_base.hpp"
 
@@ -331,6 +332,11 @@ static bool SwitchNewGRFBlitter()
 	const bool animation_wanted = HasBit(_display_opt, DO_FULL_ANIMATION);
 	const char *cur_blitter = BlitterFactory::GetCurrentBlitter()->GetName();
 
+	VideoDriver::GetInstance()->AcquireBlitterLock();
+	auto guard = scope_guard([&]() {
+		VideoDriver::GetInstance()->ReleaseBlitterLock();
+	});
+
 	for (uint i = 0; i < lengthof(replacement_blitters); i++) {
 		if (animation_wanted && (replacement_blitters[i].animation == 0)) continue;
 		if (!animation_wanted && (replacement_blitters[i].animation == 1)) continue;
diff --git a/src/video/sdl_v.cpp b/src/video/sdl_v.cpp
index 4ee96db778..c778ef27a5 100644
--- a/src/video/sdl_v.cpp
+++ b/src/video/sdl_v.cpp
@@ -830,11 +830,18 @@ bool VideoDriver_SDL::ToggleFullscreen(bool fullscreen)
 }
 
 bool VideoDriver_SDL::AfterBlitterChange()
+{
+	return CreateMainSurface(_screen.width, _screen.height);
+}
+
+void VideoDriver_SDL::AcquireBlitterLock()
 {
 	if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true);
-	bool ret = CreateMainSurface(_screen.width, _screen.height);
+}
+
+void VideoDriver_SDL::ReleaseBlitterLock()
+{
 	if (_draw_mutex != NULL) _draw_mutex->EndCritical(true);
-	return ret;
 }
 
 #endif /* WITH_SDL */
diff --git a/src/video/sdl_v.h b/src/video/sdl_v.h
index 730f82b93f..8855c3566e 100644
--- a/src/video/sdl_v.h
+++ b/src/video/sdl_v.h
@@ -31,6 +31,10 @@ public:
 
 	/* virtual */ bool AfterBlitterChange();
 
+	/* virtual */ void AcquireBlitterLock();
+
+	/* virtual */ void ReleaseBlitterLock();
+
 	/* virtual */ bool ClaimMousePointer();
 
 	/* virtual */ const char *GetName() const { return "sdl"; }
diff --git a/src/video/video_driver.hpp b/src/video/video_driver.hpp
index 916044d358..5cb3c6cc3f 100644
--- a/src/video/video_driver.hpp
+++ b/src/video/video_driver.hpp
@@ -49,6 +49,7 @@ public:
 
 	/**
 	 * Callback invoked after the blitter was changed.
+	 * This may only be called between AcquireBlitterLock and ReleaseBlitterLock.
 	 * @return True if no error.
 	 */
 	virtual bool AfterBlitterChange()
@@ -56,6 +57,18 @@ public:
 		return true;
 	}
 
+	/**
+	 * Acquire any lock(s) required to be held when changing blitters.
+	 * These lock(s) may not be acquired recursively.
+	 */
+	virtual void AcquireBlitterLock() { }
+
+	/**
+	 * Release any lock(s) required to be held when changing blitters.
+	 * These lock(s) may not be acquired recursively.
+	 */
+	virtual void ReleaseBlitterLock() { }
+
 	virtual bool ClaimMousePointer()
 	{
 		return true;
diff --git a/src/video/win32_v.cpp b/src/video/win32_v.cpp
index c37ebd7dd6..e536ae8e5e 100644
--- a/src/video/win32_v.cpp
+++ b/src/video/win32_v.cpp
@@ -1333,11 +1333,18 @@ bool VideoDriver_Win32::ToggleFullscreen(bool full_screen)
 }
 
 bool VideoDriver_Win32::AfterBlitterChange()
+{
+	return AllocateDibSection(_screen.width, _screen.height, true) && this->MakeWindow(_fullscreen);
+}
+
+void VideoDriver_Win32::AcquireBlitterLock()
 {
 	if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true);
-	bool ret = AllocateDibSection(_screen.width, _screen.height, true) && this->MakeWindow(_fullscreen);
+}
+
+void VideoDriver_Win32::ReleaseBlitterLock()
+{
 	if (_draw_mutex != NULL) _draw_mutex->EndCritical(true);
-	return ret;
 }
 
 void VideoDriver_Win32::EditBoxLostFocus()
diff --git a/src/video/win32_v.h b/src/video/win32_v.h
index 21d59185b9..7609d0422d 100644
--- a/src/video/win32_v.h
+++ b/src/video/win32_v.h
@@ -31,6 +31,10 @@ public:
 
 	/* virtual */ bool AfterBlitterChange();
 
+	/* virtual */ void AcquireBlitterLock();
+
+	/* virtual */ void ReleaseBlitterLock();
+
 	/* virtual */ bool ClaimMousePointer();
 
 	/* virtual */ void EditBoxLostFocus();