Skip to content

Commit

Permalink
Fix race conditions in ow_regcomp
Browse files Browse the repository at this point in the history 
The following trivial program will occasionally SIGSEGV, SIGABRT, or
deadlock:

  #include <owcapi.h>
  int main() {
    char* buf;
    size_t len;
    OW_init("--fake 20 --fake 42");
    OW_get("/",&buf,&len);
    return 0;
  }

Backtraces show that these occur within calls to ow_regcomp, and most
often during the underlying regcomp(3) call. It is also possible during
the tsearch(3) call, which is explicitly thread-unsafe against a shared
root.

This patch replaces the tsearch check with the standard pthread_once
mechanism, which is designed specifically for this use case.

Signed-off-by: Justin Brewer <justin.brewer@vertivco.com>
Signed-off-by: Johan Ström <johan@stromnet.se>
  • Loading branch information
@vertiv-justinbrewer @stromnet
vertiv-justinbrewer authored and stromnet committed Nov 15, 2017
1 parent 8df9cea commit 4d50ca6
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 212 deletions.
30 changes: 18 additions & 12 deletions module/owlib/src/c/ow_arg.c
View file
Expand Up @@ -23,20 +23,26 @@ enum arg_address { arg_addr_device, arg_addr_null,
arg_addr_number, arg_addr_ftdi,
arg_addr_other, arg_addr_error, } ;

static regex_t rx_dev;
static regex_t rx_num;
static regex_t rx_ip;
static regex_t rx_ftdi;
static regex_t rx_col;

static pthread_once_t regex_init_once = PTHREAD_ONCE_INIT;

static void regex_init(void)
{
ow_regcomp(&rx_dev, "/", REG_NOSUB);
ow_regcomp(&rx_num, "^[:digit:]+$", REG_NOSUB);
ow_regcomp(&rx_ip, "[:digit:]{1,3}\\.[:digit:]{1,3}\\.[:digit:]{1,3}\\.[:digit:]{1,3}", REG_NOSUB);
ow_regcomp(&rx_ftdi, "^ftdi:", REG_NOSUB);
ow_regcomp(&rx_col, ":", REG_NOSUB);
}

static enum arg_address ArgType( const char * arg )
{
static regex_t rx_dev ;
static regex_t rx_num ;
static regex_t rx_ip ;
static regex_t rx_ftdi ;
static regex_t rx_col ;

// compile regex expressions
ow_regcomp( &rx_dev, "/", REG_NOSUB ) ;
ow_regcomp( &rx_num, "^[:digit:]+$", REG_NOSUB ) ;
ow_regcomp( &rx_ip, "[:digit:]{1,3}\\.[:digit:]{1,3}\\.[:digit:]{1,3}\\.[:digit:]{1,3}", REG_NOSUB ) ;
ow_regcomp( &rx_ftdi, "^ftdi:", REG_NOSUB ) ;
ow_regcomp( &rx_col, ":", REG_NOSUB ) ;
pthread_once(&regex_init_once, regex_init);

if ( arg == NULL ) {
return arg_addr_null ;
Expand Down
3 changes: 0 additions & 3 deletions module/owlib/src/c/ow_lib_stop.c
View file
Expand Up @@ -25,9 +25,6 @@ void LibStop(void)
FreeInAll();
LEVEL_CALL("Closing output devices");
FreeOutAll();
LEVEL_CALL("Clearing compiled expressions");
ow_regdestroy() ;


/* Have to reset more internal variables, and this should be fixed
* by setting optind = 0 and call getopt()
Expand Down
47 changes: 27 additions & 20 deletions module/owlib/src/c/ow_parse_address.c
View file
Expand Up @@ -16,22 +16,34 @@
static void Init_Address( struct address_pair * ap ) ;
static void Parse_Single_Address( struct address_entry * ae ) ;

static regex_t rx_pa_none;
static regex_t rx_pa_all;
static regex_t rx_pa_scan;
static regex_t rx_pa_star;
static regex_t rx_pa_quad;
static regex_t rx_pa_num;
static regex_t rx_pa_one;
static regex_t rx_pa_two;
static regex_t rx_pa_three;

static pthread_once_t regex_init_once = PTHREAD_ONCE_INIT;

static void regex_init(void)
{
ow_regcomp(&rx_pa_none, "^$", REG_NOSUB);
ow_regcomp(&rx_pa_all, "^all$", REG_NOSUB | REG_ICASE);
ow_regcomp(&rx_pa_scan, "^scan$", REG_NOSUB | REG_ICASE);
ow_regcomp(&rx_pa_star, "^\\*$", REG_NOSUB);
ow_regcomp(&rx_pa_quad, "^[[:digit:]]{1,3}\\.[[:digit:]]{1,3}\\.[[:digit:]]{1,3}\\.[[:digit:]]{1,3}$", REG_NOSUB);
ow_regcomp(&rx_pa_num, "^-?[[:digit:]]+$", REG_NOSUB);
ow_regcomp(&rx_pa_one, "^ *([^ ]+)[ \r]*$", 0);
ow_regcomp(&rx_pa_two, "^ *([^ ]+) *: *([^ ]+)[ \r]*$", 0);
ow_regcomp(&rx_pa_three, "^ *([^ ]+) *: *([^ ]+) *: *([^ ]+)[ \r]*$", 0);
}

static void Parse_Single_Address( struct address_entry * ae )
{
static regex_t rx_pa_none ;
static regex_t rx_pa_all ;
static regex_t rx_pa_scan ;
static regex_t rx_pa_star ;
static regex_t rx_pa_quad ;
static regex_t rx_pa_num ;

ow_regcomp( &rx_pa_none, "^$", REG_NOSUB ) ;
ow_regcomp( &rx_pa_all, "^all$", REG_NOSUB | REG_ICASE ) ;
ow_regcomp( &rx_pa_scan, "^scan$", REG_NOSUB | REG_ICASE ) ;
ow_regcomp( &rx_pa_star, "^\\*$", REG_NOSUB ) ;
ow_regcomp( &rx_pa_quad, "^[[:digit:]]{1,3}\\.[[:digit:]]{1,3}\\.[[:digit:]]{1,3}\\.[[:digit:]]{1,3}$", REG_NOSUB ) ;
ow_regcomp( &rx_pa_num, "^-?[[:digit:]]+$", REG_NOSUB ) ;
pthread_once(&regex_init_once, regex_init);

if ( ae->alpha == NULL ) {
// null entry
Expand Down Expand Up @@ -67,15 +79,10 @@ static void Parse_Single_Address( struct address_entry * ae )
*/
void Parse_Address( char * address, struct address_pair * ap )
{
static regex_t rx_pa_one ;
static regex_t rx_pa_two ;
static regex_t rx_pa_three ;
pthread_once(&regex_init_once, regex_init);

struct ow_regmatch orm ;

ow_regcomp( &rx_pa_one, "^ *([^ ]+)[ \r]*$", 0 ) ;
ow_regcomp( &rx_pa_two, "^ *([^ ]+) *: *([^ ]+)[ \r]*$", 0 ) ;
ow_regcomp( &rx_pa_three, "^ *([^ ]+) *: *([^ ]+) *: *([^ ]+)[ \r]*$", 0 ) ;

// Set up address structure into previously allocated structure
if ( ap == NULL ) {
return ;
Expand Down
13 changes: 10 additions & 3 deletions module/owlib/src/c/ow_parse_sn.c
View file
Expand Up @@ -14,14 +14,21 @@
#include "owfs_config.h"
#include "ow.h"

static regex_t rx_sn_parse;

static pthread_once_t regex_init_once = PTHREAD_ONCE_INIT;

static void regex_init(void)
{
ow_regcomp(&rx_sn_parse, "^([[:xdigit:]]{2})\\.?([[:xdigit:]]{12})\\.?([[:xdigit:]]{2}){0,1}$", 0);
}

/* Fill get serikal number from a character string */
enum parse_serialnumber Parse_SerialNumber(char *sn_char, BYTE * sn)
{
static regex_t rx_sn_parse ;
pthread_once(&regex_init_once,regex_init);

struct ow_regmatch orm ;

ow_regcomp( &rx_sn_parse, "^([[:xdigit:]]{2})\\.?([[:xdigit:]]{12})\\.?([[:xdigit:]]{2}){0,1}$", 0 ) ;
orm.number = 3 ;

if ( sn_char == NULL ) {
Expand Down
123 changes: 58 additions & 65 deletions module/owlib/src/c/ow_parsename.c
View file
Expand Up @@ -52,6 +52,51 @@ static char * find_segment_in_path( char * segment, char * path ) ;

#define BRANCH_INCR (9)

static regex_t rx_bus;
static regex_t rx_set;
static regex_t rx_sta;
static regex_t rx_str;
static regex_t rx_sys;
static regex_t rx_int;
static regex_t rx_tex;
static regex_t rx_jso;
static regex_t rx_unc;
static regex_t rx_una;
static regex_t rx_ala;
static regex_t rx_sim;
static regex_t rx_the;
static regex_t rx_p_bus;
static regex_t rx_extension;
static regex_t rx_all;
static regex_t rx_byte;
static regex_t rx_number;
static regex_t rx_letter;

static pthread_once_t regex_init_once = PTHREAD_ONCE_INIT;

static void regex_init(void)
{
ow_regcomp(&rx_bus, "^bus\\.([[:digit:]]+)/?", REG_ICASE);
ow_regcomp(&rx_set, "^settings/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_sta, "^statistics/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_str, "^structure/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_sys, "^system/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_int, "^interface/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_tex, "^text/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_jso, "^json/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_unc, "^uncached/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_una, "^unaliased/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_ala, "^alarm\?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_sim, "^simultaneous/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_the, "^thermostat/?", REG_ICASE | REG_NOSUB);
ow_regcomp(&rx_p_bus, "^/bus\\.[[:digit:]]+/?", REG_ICASE);
ow_regcomp(&rx_extension, "\\.", 0);
ow_regcomp(&rx_all, "\\.all$", REG_ICASE);
ow_regcomp(&rx_byte, "\\.byte$", REG_ICASE);
ow_regcomp(&rx_number, "\\.[[:digit:]]+$", 0);
ow_regcomp(&rx_letter, "\\.[[:alpha:]]$", REG_ICASE);
}

/* ---------------------------------------------- */
/* Filename (path) parsing functions */
/* ---------------------------------------------- */
Expand Down Expand Up @@ -341,31 +386,11 @@ static enum parse_enum set_type( enum ePN_type epntype, struct parsedname * pn )
// Early parsing -- only bus entries, uncached and text may have preceeded
static enum parse_enum Parse_Unspecified(char *pathnow, enum parse_pass remote_status, struct parsedname *pn)
{
static regex_t rx_bus ;
static regex_t rx_set ;
static regex_t rx_sta ;
static regex_t rx_str ;
static regex_t rx_sys ;
static regex_t rx_int ;
static regex_t rx_tex ;
static regex_t rx_jso ;
static regex_t rx_unc ;
static regex_t rx_una ;

pthread_once(&regex_init_once, regex_init);

struct ow_regmatch orm ;
orm.number = 1 ; // for bus

ow_regcomp( &rx_bus, "^bus\\.([[:digit:]]+)/?", REG_ICASE ) ;
ow_regcomp( &rx_set, "^settings/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_sta, "^statistics/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_str, "^structure/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_sys, "^system/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_int, "^interface/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_tex, "^text/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_jso, "^json/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_unc, "^uncached/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_una, "^unaliased/?", REG_ICASE | REG_NOSUB ) ;


if ( ow_regexec( &rx_bus, pathnow, &orm ) == 0) {
INDEX_OR_ERROR bus_number = (INDEX_OR_ERROR) atoi(orm.match[1]) ;
ow_regexec_free( &orm ) ;
Expand Down Expand Up @@ -414,10 +439,8 @@ static enum parse_enum Parse_Unspecified(char *pathnow, enum parse_pass remote_s

static enum parse_enum Parse_Branch(char *pathnow, enum parse_pass remote_status, struct parsedname *pn)
{
static regex_t rx_ala ;

ow_regcomp( &rx_ala, "^alarm\?", REG_ICASE | REG_NOSUB ) ;

pthread_once(&regex_init_once, regex_init);

if (ow_regexec( &rx_ala, pathnow, NULL ) == 0) {
pn->state |= ePS_alarm;
pn->type = ePN_real;
Expand All @@ -428,20 +451,8 @@ static enum parse_enum Parse_Branch(char *pathnow, enum parse_pass remote_status

static enum parse_enum Parse_Real(char *pathnow, enum parse_pass remote_status, struct parsedname *pn)
{
static regex_t rx_sim ;
static regex_t rx_the ;
static regex_t rx_tex ;
static regex_t rx_jso ;
static regex_t rx_unc ;
static regex_t rx_una ;

ow_regcomp( &rx_sim, "^simultaneous/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_the, "^thermostat/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_tex, "^text/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_jso, "^json/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_unc, "^uncached/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_una, "^unaliased/?", REG_ICASE | REG_NOSUB ) ;

pthread_once(&regex_init_once, regex_init);

if (ow_regexec( &rx_sim, pathnow, NULL ) == 0) {
pn->selected_device = DeviceSimultaneous;
return parse_prop;
Expand Down Expand Up @@ -473,15 +484,7 @@ static enum parse_enum Parse_Real(char *pathnow, enum parse_pass remote_status,

static enum parse_enum Parse_NonReal(char *pathnow, struct parsedname *pn)
{
static regex_t rx_tex ;
static regex_t rx_jso ;
static regex_t rx_unc ;
static regex_t rx_una ;

ow_regcomp( &rx_tex, "^text/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_jso, "^json/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_unc, "^uncached/?", REG_ICASE | REG_NOSUB ) ;
ow_regcomp( &rx_una, "^unaliased/?", REG_ICASE | REG_NOSUB ) ;
pthread_once(&regex_init_once, regex_init);

if (ow_regexec( &rx_tex, pathnow, NULL ) == 0) {
pn->state |= ePS_text;
Expand Down Expand Up @@ -509,10 +512,9 @@ static enum parse_enum Parse_NonReal(char *pathnow, struct parsedname *pn)
/* We've reached a /bus.n entry */
static enum parse_enum Parse_Bus( INDEX_OR_ERROR bus_number, struct parsedname *pn)
{
static regex_t rx_p_bus ;
pthread_once(&regex_init_once, regex_init);

struct ow_regmatch orm ;

ow_regcomp( &rx_p_bus, "^/bus\\.[[:digit:]]+/?", REG_ICASE ) ;
orm.number = 0 ;

/* Processing for bus.X directories -- eventually will make this more generic */
Expand Down Expand Up @@ -728,25 +730,16 @@ static enum parse_enum Parse_NonRealDevice(char *filename, struct parsedname *pn

static enum parse_enum Parse_Property(char *filename, struct parsedname *pn)
{
pthread_once(&regex_init_once, regex_init);

struct device * pdev = pn->selected_device ;
struct filetype * ft ;

static regex_t rx_extension ;
static regex_t rx_all ;
static regex_t rx_byte ;
static regex_t rx_number ;
static regex_t rx_letter ;
int extension_given ;

struct ow_regmatch orm ;
orm.number = 0 ;

ow_regcomp( &rx_extension, "\\.", 0 ) ;
ow_regcomp( &rx_all, "\\.all$", REG_ICASE ) ;
ow_regcomp( &rx_byte, "\\.byte$", REG_ICASE ) ;
ow_regcomp( &rx_number, "\\.[[:digit:]]+$", 0 ) ;
ow_regcomp( &rx_letter, "\\.[[:alpha:]]$", REG_ICASE ) ;


//printf("FilePart: %s %s\n", filename, pn->path);

// Special case for remote device. Use distant data
Expand Down

0 comments on commit 4d50ca6

Please sign in to comment.