[Skiboot] [PATCH] Fix race in flash resource preloading

[Alistair Popple]

From: Stewart Smith <stewart at linux.vnet.ibm.com>

(13:31:46) benh: stewart: flash_load_resources()
(13:31:53) benh: stewart: you hit the unlock at the bottom of the loop
(13:31:59) benh: stewart: at that point the list may be empty
(13:32:09) benh: stewart: and so another concurrent load can restart the thread
(13:32:15) benh: stewart: you end up with duplicate threads
(13:32:26) benh: stewart: in which case you can hit the assert
<patch goes here>
(13:34:27) benh: ie, never drop the lock with the queue empty
(13:34:29) benh: unless you know you will exit the job
(13:34:32) benh: otherwise you can have a duplicate job
(13:34:41) benh: -> kaboom

(13:36:29) benh: yeah the decision to exit the loop must be atomic with
		 the popping of the last element in the list
(13:36:43) benh: to match the decision to start the thread which is atomic
		 with the queuing of the first element

Reported-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
 core/flash.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/core/flash.c b/core/flash.c
index 092f8f2..c08bb49 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -680,15 +680,16 @@ static void flash_load_resources(void *data __unused)
 	struct flash_load_resource_item *r;
 	int result;
 
+	lock(&flash_load_resource_lock);
 	do {
-		lock(&flash_load_resource_lock);
 		if (list_empty(&flash_load_resource_queue)) {
-			unlock(&flash_load_resource_lock);
 			break;
 		}
 		r = list_top(&flash_load_resource_queue,
 			     struct flash_load_resource_item, link);
-		assert(r->result == OPAL_EMPTY);
+		if (r->result != OPAL_EMPTY)
+			prerror("flash_load_resources() list_top unexpected "
+				" result %d\n", r->result);
 		r->result = OPAL_BUSY;
 		unlock(&flash_load_resource_lock);
 
@@ -699,8 +700,8 @@ static void flash_load_resources(void *data __unused)
 			     struct flash_load_resource_item, link);
 		r->result = result;
 		list_add_tail(&flash_loaded_resources, &r->link);
-		unlock(&flash_load_resource_lock);
 	} while(true);
+	unlock(&flash_load_resource_lock);
 }
 
 static void start_flash_load_resource_job(void)
-- 
1.8.3.2