[rtems commit] score: Fix race condition in SMP startup

Sebastian Huber sebh at rtems.org
Mon May 26 07:02:22 UTC 2014 

Module:    rtems
Branch:    master
Commit:    d134adeba3abd0fef8ed2046cc5bf7f877d7a77c
Changeset: http://git.rtems.org/rtems/commit/?id=d134adeba3abd0fef8ed2046cc5bf7f877d7a77c

Author:    Sebastian Huber <sebastian.huber at embedded-brains.de>
Date:      Fri May 23 15:32:32 2014 +0200

score: Fix race condition in SMP startup

Do not use the Per_CPU_Control::started in
_SMP_Start_multitasking_on_secondary_processor() since this field may be
not up to date when a secondary processor reads it.  Use the read-only
scheduler assignment instead.

Add a new fatal error SMP_FATAL_MULTITASKING_START_ON_INVALID_PROCESSOR.
This prevents out-of-bounds access.

It is currently not possible to test these fatal errors.  One option
would be to fake values of the _CPU_SMP_Get_current_processor(), but
unfortunately this function is inline on some architectures.

---

 cpukit/score/include/rtems/score/smpimpl.h    |    3 +-
 cpukit/score/src/smp.c                        |    9 +++-
 testsuites/smptests/Makefile.am               |    1 -
 testsuites/smptests/configure.ac              |    1 -
 testsuites/smptests/smpfatal07/Makefile.am    |   19 ------
 testsuites/smptests/smpfatal07/init.c         |   75 -------------------------
 testsuites/smptests/smpfatal07/smpfatal07.doc |   12 ----
 testsuites/smptests/smpfatal07/smpfatal07.scn |    2 -
 8 files changed, 10 insertions(+), 112 deletions(-)

diff --git a/cpukit/score/include/rtems/score/smpimpl.h b/cpukit/score/include/rtems/score/smpimpl.h
index 3ee7aa2..70475ba 100644
--- a/cpukit/score/include/rtems/score/smpimpl.h
+++ b/cpukit/score/include/rtems/score/smpimpl.h
@@ -60,7 +60,8 @@ typedef enum {
   SMP_FATAL_MANDATORY_PROCESSOR_NOT_PRESENT,
   SMP_FATAL_MULTITASKING_START_ON_UNASSIGNED_PROCESSOR,
   SMP_FATAL_START_OF_MANDATORY_PROCESSOR_FAILED,
-  SMP_FATAL_SHUTDOWN_RESPONSE
+  SMP_FATAL_SHUTDOWN_RESPONSE,
+  SMP_FATAL_MULTITASKING_START_ON_INVALID_PROCESSOR
 } SMP_Fatal_code;
 
 static inline void _SMP_Fatal( SMP_Fatal_code code )
diff --git a/cpukit/score/src/smp.c b/cpukit/score/src/smp.c
index b9d4b0a..f0554fe 100644
--- a/cpukit/score/src/smp.c
+++ b/cpukit/score/src/smp.c
@@ -121,8 +121,15 @@ void _SMP_Request_start_multitasking( void )
 void _SMP_Start_multitasking_on_secondary_processor( void )
 {
   Per_CPU_Control *self_cpu = _Per_CPU_Get();
+  uint32_t cpu_index_self = _Per_CPU_Get_index( self_cpu );
+  const Scheduler_Assignment *assignment =
+    _Scheduler_Get_assignment( cpu_index_self );
 
-  if ( !_Per_CPU_Is_processor_started( self_cpu ) ) {
+  if ( cpu_index_self >= rtems_configuration_get_maximum_processors() ) {
+    _SMP_Fatal( SMP_FATAL_MULTITASKING_START_ON_INVALID_PROCESSOR );
+  }
+
+  if ( !_Scheduler_Should_start_processor( assignment ) ) {
     _SMP_Fatal( SMP_FATAL_MULTITASKING_START_ON_UNASSIGNED_PROCESSOR );
   }
 
diff --git a/testsuites/smptests/Makefile.am b/testsuites/smptests/Makefile.am
index ed6fefd..4a5a78b 100644
--- a/testsuites/smptests/Makefile.am
+++ b/testsuites/smptests/Makefile.am
@@ -16,7 +16,6 @@ SUBDIRS += smpfatal02
 SUBDIRS += smpfatal03
 SUBDIRS += smpfatal04
 SUBDIRS += smpfatal05
-SUBDIRS += smpfatal07
 SUBDIRS += smpfatal08
 SUBDIRS += smpipi01
 SUBDIRS += smpload01
diff --git a/testsuites/smptests/configure.ac b/testsuites/smptests/configure.ac
index f9e7662..607adbb 100644
--- a/testsuites/smptests/configure.ac
+++ b/testsuites/smptests/configure.ac
@@ -71,7 +71,6 @@ smpfatal02/Makefile
 smpfatal03/Makefile
 smpfatal04/Makefile
 smpfatal05/Makefile
-smpfatal07/Makefile
 smpfatal08/Makefile
 smpipi01/Makefile
 smpload01/Makefile
diff --git a/testsuites/smptests/smpfatal07/Makefile.am b/testsuites/smptests/smpfatal07/Makefile.am
deleted file mode 100644
index 724ff93..0000000
--- a/testsuites/smptests/smpfatal07/Makefile.am
+++ /dev/null
@@ -1,19 +0,0 @@
-rtems_tests_PROGRAMS = smpfatal07
-smpfatal07_SOURCES = init.c
-
-dist_rtems_tests_DATA = smpfatal07.scn smpfatal07.doc
-
-include $(RTEMS_ROOT)/make/custom/@RTEMS_BSP at .cfg
-include $(top_srcdir)/../automake/compile.am
-include $(top_srcdir)/../automake/leaf.am
-
-AM_CPPFLAGS += -I$(top_srcdir)/../support/include
-
-LINK_OBJS = $(smpfatal07_OBJECTS)
-LINK_LIBS = $(smpfatal07_LDLIBS)
-
-smpfatal07$(EXEEXT): $(smpfatal07_OBJECTS) $(smpfatal07_DEPENDENCIES)
-	@rm -f smpfatal07$(EXEEXT)
-	$(make-exe)
-
-include $(top_srcdir)/../automake/local.am
diff --git a/testsuites/smptests/smpfatal07/init.c b/testsuites/smptests/smpfatal07/init.c
deleted file mode 100644
index 1262742..0000000
--- a/testsuites/smptests/smpfatal07/init.c
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * Copyright (c) 2014 embedded brains GmbH.  All rights reserved.
- *
- *  embedded brains GmbH
- *  Dornierstr. 4
- *  82178 Puchheim
- *  Germany
- *  <rtems at embedded-brains.de>
- *
- * The license and distribution terms for this file may be
- * found in the file LICENSE in this distribution or at
- * http://www.rtems.org/license/LICENSE.
- */
-
-#ifdef HAVE_CONFIG_H
-  #include "config.h"
-#endif
-
-#include <rtems.h>
-#include <rtems/test.h>
-#include <rtems/score/smpimpl.h>
-#include <rtems/score/threadimpl.h>
-
-#include <assert.h>
-#include <stdlib.h>
-
-const char rtems_test_name[] = "SMPFATAL 7";
-
-static void Init(rtems_task_argument arg)
-{
-  Per_CPU_Control *cpu_self;
-
-  _Thread_Disable_dispatch();
-  cpu_self = _Per_CPU_Get();
-  cpu_self->started = false;
-  _SMP_Start_multitasking_on_secondary_processor();
-
-  assert(0);
-}
-
-static void fatal_extension(
-  rtems_fatal_source source,
-  bool is_internal,
-  rtems_fatal_code code
-)
-{
-  rtems_test_begink();
-
-  if (
-    source == RTEMS_FATAL_SOURCE_SMP
-      && !is_internal
-      && code == SMP_FATAL_MULTITASKING_START_ON_UNASSIGNED_PROCESSOR
-  ) {
-    rtems_test_endk();
-  }
-}
-
-#define CONFIGURE_APPLICATION_DOES_NOT_NEED_CLOCK_DRIVER
-#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
-
-#define CONFIGURE_INITIAL_EXTENSIONS \
-  { .fatal = fatal_extension }, \
-  RTEMS_TEST_INITIAL_EXTENSION
-
-#define CONFIGURE_SMP_APPLICATION
-
-#define CONFIGURE_SMP_MAXIMUM_PROCESSORS 1
-
-#define CONFIGURE_MAXIMUM_TASKS 1
-
-#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
-
-#define CONFIGURE_INIT
-
-#include <rtems/confdefs.h>
diff --git a/testsuites/smptests/smpfatal07/smpfatal07.doc b/testsuites/smptests/smpfatal07/smpfatal07.doc
deleted file mode 100644
index 5f37225..0000000
--- a/testsuites/smptests/smpfatal07/smpfatal07.doc
+++ /dev/null
@@ -1,12 +0,0 @@
-This file describes the directives and concepts tested by this test set.
-
-test set name: smpfatal07
-
-directives:
-
-  - _SMP_Start_multitasking_on_secondary_processor()
-
-concepts:
-
-  - Ensure that a multitasking start on an unassigned processor leads to a
-    fatal error.
diff --git a/testsuites/smptests/smpfatal07/smpfatal07.scn b/testsuites/smptests/smpfatal07/smpfatal07.scn
deleted file mode 100644
index 10252a6..0000000
--- a/testsuites/smptests/smpfatal07/smpfatal07.scn
+++ /dev/null
@@ -1,2 +0,0 @@
-*** TEST SMPFATAL 7 ***
-*** END OF TEST SMPFATAL 7 ***

More information about the vc mailing list