Skip to content

Commit

Permalink
[usb-moded] Provide thread safe access to mode data list. Fixes JB#45312
Browse files Browse the repository at this point in the history
While the worker thread already caches private copy of mode data,
acquiring this private copy is hazardous due to iterating list owned
by the main thread.

Add usbmoded_dup_modedata() function that does the list iteration and
duplication while holding mutex, and utilize it for looking up mode
data from the worker thread.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
  • Loading branch information
spiiroin committed Apr 9, 2019
1 parent d3df694 commit b5a8e8b
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/usb_moded-worker.c
Expand Up @@ -579,6 +579,7 @@ worker_switch_to_mode(const char *mode)
LOG_REGISTER_CONTEXT;

const char *override = 0;
modedata_t *data = 0;

/* set return to 1 to be sure to error out if no matching mode is found either */

Expand Down Expand Up @@ -612,8 +613,7 @@ worker_switch_to_mode(const char *mode)
goto FAILED;
}

const modedata_t *data = usbmoded_get_modedata(mode);
if( data ) {
if( (data = usbmoded_dup_modedata(mode)) ) {
log_debug("Matching mode %s found.\n", mode);

/* set data before calling any of the dynamic mode functions
Expand Down Expand Up @@ -704,6 +704,8 @@ worker_switch_to_mode(const char *mode)

worker_notify();

modedata_free(data);

return;
}

Expand Down
77 changes: 77 additions & 0 deletions src/usb_moded.c
Expand Up @@ -111,6 +111,7 @@ GList *usbmoded_get_modelist (void);
void usbmoded_load_modelist (void);
void usbmoded_free_modelist (void);
const modedata_t *usbmoded_get_modedata (const char *modename);
modedata_t *usbmoded_dup_modedata (const char *modename);
bool usbmoded_get_rescue_mode (void);
void usbmoded_set_rescue_mode (bool rescue_mode);
bool usbmoded_get_diag_mode (void);
Expand Down Expand Up @@ -150,6 +151,22 @@ static bool usbmoded_systemd_notify = false;
#endif
static bool usbmoded_auto_exit = false;

static pthread_mutex_t usbmoded_mutex = PTHREAD_MUTEX_INITIALIZER;

#define USBMODED_LOCKED_ENTER do {\
if( pthread_mutex_lock(&usbmoded_mutex) != 0 ) { \
log_crit("USBMODED LOCK FAILED");\
_exit(EXIT_FAILURE);\
}\
}while(0)

#define USBMODED_LOCKED_LEAVE do {\
if( pthread_mutex_unlock(&usbmoded_mutex) != 0 ) { \
log_crit("USBMODED UNLOCK FAILED");\
_exit(EXIT_FAILURE);\
}\
}while(0)

/* ========================================================================= *
* Functions
* ========================================================================= */
Expand All @@ -158,8 +175,18 @@ static bool usbmoded_auto_exit = false;
* MODELIST
* ------------------------------------------------------------------------- */

/** List of mode data items read from configuration files
*
* Note: Worker thread should access this only via #usbmoded_dup_modedata().
*/
static GList *usbmoded_modelist = 0;

/** Get list of dynamic mode data items
*
* Note: This function should be called only from the main thread.
*
* @returns List of mode data objects, or NULL
*/
GList *
usbmoded_get_modelist(void)
{
Expand All @@ -168,32 +195,58 @@ usbmoded_get_modelist(void)
return usbmoded_modelist;
}

/** Load dynamic mode data items
*
* Note: This function should be called only from the main thread.
*/
void
usbmoded_load_modelist(void)
{
LOG_REGISTER_CONTEXT;

USBMODED_LOCKED_ENTER;

if( !usbmoded_modelist ) {
log_notice("load modelist");
usbmoded_modelist = modelist_load(usbmoded_get_diag_mode());
}

USBMODED_LOCKED_LEAVE;
}

/** Free dynamic mode data items
*
* Note: This function should be called only from the main thread.
*/
void
usbmoded_free_modelist(void)
{
LOG_REGISTER_CONTEXT;

USBMODED_LOCKED_ENTER;

if( usbmoded_modelist ) {
log_notice("free modelist");
modelist_free(usbmoded_modelist),
usbmoded_modelist = 0;
}

USBMODED_LOCKED_LEAVE;
}

/** Lookup dynamic mode data by name
*
* Note: This function should be called only from the main thread.
*
* @param modename Name of mode to lookup
*
* @return Mode data object, or NULL
*/
const modedata_t *
usbmoded_get_modedata(const char *modename)
{
LOG_REGISTER_CONTEXT;

modedata_t *modedata = 0;

for( GList *iter = usbmoded_get_modelist(); iter; iter = g_list_next(iter) ) {
Expand All @@ -206,6 +259,30 @@ usbmoded_get_modedata(const char *modename)
return modedata;
}

/** Lookup and clone dynamic mode data by name
*
* Note: This function is safe to call from worker thread too.
*
* Caller must release the returned object via #modedata_free().
*
* @param modename Name of mode to lookup
*
* @return Mode data object, or NULL
*/
modedata_t *
usbmoded_dup_modedata(const char *modename)
{
LOG_REGISTER_CONTEXT;

USBMODED_LOCKED_ENTER;

modedata_t *modedata = modedata_copy(usbmoded_get_modedata(modename));

USBMODED_LOCKED_LEAVE;

return modedata;
}

/* ------------------------------------------------------------------------- *
* RESCUE_MODE
* ------------------------------------------------------------------------- */
Expand Down
1 change: 1 addition & 0 deletions src/usb_moded.h
Expand Up @@ -70,6 +70,7 @@ GList *usbmoded_get_modelist (void);
void usbmoded_load_modelist (void);
void usbmoded_free_modelist (void);
const modedata_t *usbmoded_get_modedata (const char *modename);
modedata_t *usbmoded_dup_modedata (const char *modename);
bool usbmoded_get_rescue_mode (void);
void usbmoded_set_rescue_mode (bool rescue_mode);
bool usbmoded_get_diag_mode (void);
Expand Down

0 comments on commit b5a8e8b

Please sign in to comment.