flexnbd status: Avoid a possible NULL dereference reading migration status

While the mirror mutex is taken, the mirroring can be abandoned and serve->mirror
set to NULL, so we need to lock around reading information from serve->mirror

Rakefile

@@ -195,6 +195,30 @@ file check("serve") =>
   gcc_link t.name, t.prerequisites + [LIBCHECK]
 end
 
+file check("status") =>
+%w{
+build/tests/check_status.o
+  build/self_pipe.o
+  build/nbdtypes.o
+  build/control.o
+  build/readwrite.o
+  build/parse.o
+  build/client.o
+  build/flexthread.o
+  build/serve.o
+  build/flexnbd.o
+  build/mirror.o
+  build/status.o
+  build/acl.o
+  build/mbox.o
+  build/ioutil.o
+  build/sockutil.o
+  build/util.o
+} do |t|
+  gcc_link t.name, t.prerequisites + [LIBCHECK]
+end
+
+
 file check("readwrite") =>
 %w{build/tests/check_readwrite.o
   build/readwrite.o
@@ -244,7 +268,7 @@ file check("control") =>
   gcc_link t.name, t.prerequisites + [LIBCHECK]
 end
 
-(TEST_MODULES- %w{control flexnbd acl client serve readwrite util}).each do |m|
+(TEST_MODULES- %w{status control flexnbd acl client serve readwrite util}).each do |m|
   tgt = "build/tests/check_#{m}.o"
   maybe_obj_name = "build/#{m}.o"
   # Take it out in case we're testing one of the utils

src/status.c

@@ -11,12 +11,16 @@ struct status * status_create( struct server * serve )
 	status->pid = getpid();
 	status->size = serve->size;
 	status->has_control = serve->success;
-	status->is_mirroring = NULL != serve->mirror;
 
+	server_lock_start_mirror( serve );
+
+	status->is_mirroring = NULL != serve->mirror;
 	if ( status->is_mirroring ) {
 		status->migration_pass = serve->mirror->pass;
 	}
 
+	server_unlock_start_mirror( serve );
+
 	return status;
 
 }

tests/unit/check_status.c

@@ -5,102 +5,117 @@
 
 #include <check.h>
 
-void prepare_server( struct server* serve )
+struct server* mock_server(void)
 {
-	serve->mirror = NULL;
+	struct server* out = xmalloc( sizeof( struct server ) );
+	out->l_start_mirror = flexthread_mutex_create();
+	return out;
+}
+
+struct server* mock_mirroring_server(void)
+{
+	struct server *out = mock_server();
+	out->mirror = xmalloc( sizeof( struct mirror ) );
+
+	return out;
+}
+
+void destroy_mock_server( struct server* serve )
+{
+	if ( NULL != serve->mirror ) {
+		free( serve->mirror );
+	}
+
+	flexthread_mutex_destroy( serve->l_start_mirror );
+	free( serve );
 }
 
 START_TEST( test_status_create )
 {
-	struct server server;
+	struct server * server = mock_server();
-	struct status *status = NULL;
+	struct status * status = status_create( server );
-
-	prepare_server( &server );
-	status = status_create( &server );
 
 	fail_if( NULL == status, "Status wasn't allocated" );
 	status_destroy( status );
+	destroy_mock_server( server );
 }
 END_TEST
 
 START_TEST( test_gets_has_control )
 {
-	struct server server;
+	struct server * server = mock_server();
-	struct status * status;
+	server->success = 1;
 
-	prepare_server( &server );
+	struct status * status = status_create( server );
-	server.success = 1;
-	status = status_create( &server );
 
 	fail_unless( status->has_control == 1, "has_control wasn't copied" );
 	status_destroy( status );
+	destroy_mock_server( server );
 }
 END_TEST
 
 
 START_TEST( test_gets_is_mirroring )
 {
-	struct server server;
+	struct server * server = mock_server();
-	struct status * status;
+	struct status * status = status_create( server );
 
-	prepare_server( &server );
-	status = status_create( &server );
 	fail_if( status->is_mirroring, "is_mirroring was set" );
 	status_destroy( status );
+	destroy_mock_server( server );
+
+	server = mock_mirroring_server();
+	status = status_create( server );
 
-	server.mirror = (struct mirror *)xmalloc( sizeof( struct mirror ) );
-	status = status_create( &server );
 	fail_unless( status->is_mirroring, "is_mirroring wasn't set" );
 	status_destroy( status );
+	destroy_mock_server( server );
 }
 END_TEST
 
 START_TEST( test_gets_migration_pass )
 {
-	struct server server;
+	struct server * server = mock_server();
-	struct status * status;
+	struct status * status = status_create( server );
 
-	prepare_server( &server );
-	status = status_create( &server );
 	fail_if( status->migration_pass != 0, "migration_pass was set" );
 	status_destroy( status );
+	destroy_mock_server( server );
+
+	server = mock_mirroring_server();
+	server->mirror->pass = 1;
+	status = status_create( server );
 
-	server.mirror = (struct mirror *)xmalloc( sizeof( struct mirror ) );
-	server.mirror->pass = 1;
-	status = status_create( &server );
 	fail_unless( status->migration_pass == 1, "migration_pass wasn't set" );
 	status_destroy( status );
-	free( server.mirror );
+	destroy_mock_server( server );
 }
 END_TEST
 
 
 START_TEST( test_gets_pid )
 {
-	struct server server;
+	struct server * server = mock_server();
-	struct status * status;
+	struct status * status = status_create( server );
-
-	prepare_server( &server );
-	status = status_create( &server );
 
 	fail_unless( getpid() == status->pid, "Pid wasn't gathered" );
 
 	status_destroy( status );
+	destroy_mock_server( server );
 }
 END_TEST
 
 START_TEST( test_gets_size )
 {
-	struct server server;
+	struct server * server = mock_server();
-	struct status * status;
+	server->size = 1024;
 
-	prepare_server( &server );
+	struct status * status = status_create( server );
-	server.size = 1024;
-	status = status_create( &server );
 
 	fail_unless( 1024 == status->size, "Size wasn't gathered" );
 
 	status_destroy( status );
+	destroy_mock_server( server );
 }
 END_TEST
 
@@ -238,6 +253,8 @@ Suite *status_suite(void)
 	tcase_add_test(tc_create, test_gets_pid);
 	tcase_add_test(tc_create, test_gets_size);
 	tcase_add_test(tc_create, test_gets_migration_pass);
+	tcase_add_test(tc_create, test_gets_pass_statistics);
+
 
 	tcase_add_test(tc_render, test_renders_has_control);
 	tcase_add_test(tc_render, test_renders_is_mirroring);