Menu

#1416 Potential race condition during startup.

2.13.3
closed-fixed
5
2009-02-12
2009-01-28
Ric White
No

There is a potential for a race condition when the event threads are spawned during plug-in startup. This problem was discovered via code inspection, and has not been reported during execution.

In oa_soap_discover_resources(). The two local variables
oa1_event_handler and oa2_event_handler will be allocated on the stack. Their addresses are then passed to the event threads as parameters. There is no guarantee that the call to oa_soap_discover_resources() will still be in progress when these threads finally run and de-reference their parameters. If oa_soap_discover_resources() has returned before these threads execute, a call to some other function may be using the same locations on the stack, and could corrupt the contents of oa1_event_handler and oa2_event_handler before the threads have read their data. I think the parameters to the threads should be elements in the struct oa_soap_handler, rather than stack variables.

Discussion

  • Ric White

    Ric White - 2009-01-28
    • assigned_to: sutula --> ricwhite
     
  • Ric White

    Ric White - 2009-02-12

    Description of fix:

    The glib threads can only be passed a single gpointer argument when they are
    spawned. The existing implementation needs to pass two arguments to the
    event threads as parameters when they are started. The following structure
    is used for this:

    struct event_handler {
    struct oh_handler_state *oh_handler;
    struct oa_info *oa;
    }

    In oa_soap_discover_resources(), two local variables of type struct
    event_handler are used to contain the parameters for the threads. For example,
    the parameters for the OA #1 thread are set by:

    oa1_event_handler.oh_handler = handler;
    oa1_event_handler.oa = oa_handler->oa_1;

    My proposed solution is to add an element in struct oa_info to contain the
    handler pointer, and then just pass the address of the oa_handler->oa_1,
    and oa_handler->oa_2 structures to the threads.

    The changes required are:

    oa_soap.h:
    - Add an element 'struct oh_handler_state *oh_handler' to struct
    oa_info.
    - Remove the obsolete definition of struct event_handler.

    oa_soap.c: build_oa_soap_custom_handler():
    - Here is where the oa_handler->oa_1, and oa_handler->oa_2 structures
    are allocated and initialized. Set the new oh_handler element in
    these structures to the oh_handler that has been passed as a
    parameter to build_oa_soap_custom_handler().

    oa_soap_discover.c: oa_soap_discover_resources():
    - Remove the local variables oa1_event_handler and oa2_event_handler.
    - Spawn the OA #1 thread with oa_handler->oa_1 as a parameter, and
    OA #2 thread with oa_handler->oa_2 as a parameter.

    oa_soap_event.c: oa_soap_event_thread():
    - Make changes here to extract the oa, handler, and oa_handler
    parameters from the oa_info pointer parameter, rather than from
    the former event_handler pointer parameter.

    oa_soap_event.h:
    - Fixup the prototype for oa_soap_event_thread(), to reflect the
    parameter change. (not actually necessary, but makes things more
    consistent.

    File Added: oa_soap_2543767.patch

     
  • Ric White

    Ric White - 2009-02-12

    Here is a patch for the problem

     
  • Ric White

    Ric White - 2009-02-12
    • status: open --> closed-fixed
     
  • Ric White

    Ric White - 2009-02-12

    Fixed in revision 6953.

     
MongoDB Logo MongoDB