From a338c2f0f98baa44bd0549d9695fbeaf28f53713 Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Thu, 29 Mar 2012 14:07:07 +1030 Subject: [PATCH] Refactor instance path handling - handle buffer limits when forming path names within instance dir - uniform use of serval_instancepath() --- commandline.c | 119 +++++++++++++++++++++----------------------- dna.c | 24 ++++++--- overlay_interface.c | 5 +- overlay_mdp.c | 40 ++++++--------- rhizome_bundle.c | 5 ++ serval.h | 7 +++ server.c | 6 +-- 7 files changed, 106 insertions(+), 100 deletions(-) diff --git a/commandline.c b/commandline.c index 3938115d..5825cfb2 100644 --- a/commandline.c +++ b/commandline.c @@ -31,12 +31,9 @@ typedef struct command_line_option { extern command_line_option command_line_options[]; -int servalNodeRunning(int *pid,char *instancepath) +static int servalNodeRunning(int *pid) { - char filename[1024]; - char line[1024]; - if (!instancepath) instancepath=DEFAULT_INSTANCE_PATH; - + char *instancepath = serval_instancepath(); struct stat st; int r=stat(instancepath,&st); if (r) { @@ -57,20 +54,23 @@ int servalNodeRunning(int *pid,char *instancepath) } int running=0; - snprintf(filename,1023,"%s/serval.pid",instancepath); filename[1023]=0; - FILE *f=fopen(filename,"r"); - if (f) { - line[0]=0; fgets(line,1024,f); - *pid = strtoll(line,NULL,10); - running=*pid; - if (running) { - /* Check that process is really running. - Some systems don't have /proc (including mac), - so we need to find out some otherway.*/ - running=1; // assume pid means is running for now - } - fclose(f); - } + char filename[1024]; + if (FORM_SERVAL_INSTANCE_PATH(filename, "serval.pid")) { + FILE *f=fopen(filename,"r"); + if (f) { + char line[1024]; + line[0]=0; fgets(line,1024,f); + *pid = strtoll(line,NULL,10); + running=*pid; + if (running) { + /* Check that process is really running. + Some systems don't have /proc (including mac), + so we need to find out some otherway.*/ + running=1; // assume pid means is running for now + } + fclose(f); + } + } return running; } @@ -170,12 +170,16 @@ char *confValueGet(char *var,char *defaultValue) if (!var) return defaultValue; int varLen=strlen(var); - char *instancepath=serval_instancepath(); - char filename[1024]; - snprintf(filename,1024,"%s/serval.conf",instancepath); filename[1023]=0; - FILE *f=fopen(filename,"r"); - if (!f) return defaultValue; + if (!FORM_SERVAL_INSTANCE_PATH(filename, "serval.conf")) { + fprintf(stderr, "Using default value of %s: %s\n", var, defaultValue); + return defaultValue; + } + FILE *f = fopen(filename,"r"); + if (!f) { + fprintf(stderr, "Cannot open serval.conf. Using default value of %s: %s\n", var, defaultValue); + return defaultValue; + } char line[1024]; line[0]=0; fgets(line,1024,f); @@ -224,7 +228,7 @@ int app_server_start(int argc,char **argv,struct command_line_option *o) setVerbosity(confValueGet("debug","")); int pid=-1; - int running = servalNodeRunning(&pid,instancepath); + int running = servalNodeRunning(&pid); if (running<0) return -1; if (running>0) { fprintf(stderr,"ERROR: Serval process already running (pid=%d)\n",pid); @@ -238,12 +242,7 @@ int app_server_start(int argc,char **argv,struct command_line_option *o) */ rhizome_datastore_path=strdup(instancepath); rhizome_opendb(); - char temp[1024];temp[1023]=0; - snprintf(temp,1024,"%s/hlr.dat",instancepath); - if (temp[1023]) { - exit(WHY("Instance path directory name too long.")); - } - char *hlr_file=strdup(temp); + char *hlr_file = asprintf("%s/%s", instancepath, "hlr.dat"); hlr_size=atof(confValueGet("hlr_size","1"))*1048576; if (hlr_size<0) { fprintf(stderr,"HLR Size must be >0MB\n"); @@ -256,11 +255,10 @@ int app_server_start(int argc,char **argv,struct command_line_option *o) int app_server_stop(int argc,char **argv,struct command_line_option *o) { - char *instancepath=getenv("SERVALINSTANCE_PATH"); - if (!instancepath) instancepath=DEFAULT_INSTANCE_PATH; + thisinstancepath = cli_arg(argc,argv,o,"instance path",getenv("SERVALINSTANCE_PATH")); int pid=-1; - int running = servalNodeRunning(&pid,instancepath); + int running = servalNodeRunning(&pid); if (running>0) { /* Is running, so we can try to kill it. This is a little complicated by the fact that we catch most signals @@ -275,9 +273,8 @@ int app_server_stop(int argc,char **argv,struct command_line_option *o) } char stopfile[1024]; - snprintf(stopfile,1024,"%s/doshutdown",instancepath); - FILE *f=fopen(stopfile,"w"); - if (!f) { + FILE *f; + if (!(FORM_SERVAL_INSTANCE_PATH(stopfile, "doshutdown") && (f = fopen(stopfile, "w")))) { WHY("Could not create shutdown file"); return -1; } @@ -305,7 +302,7 @@ int app_server_stop(int argc,char **argv,struct command_line_option *o) time_t timeout=time(0)+5; while(timeout>time(0)) { pid=-1; - int running = servalNodeRunning(&pid,instancepath); + int running = servalNodeRunning(&pid); if (running<1) { fprintf(stderr,"Serval process appears to have stopped.\n"); return 0; @@ -322,18 +319,13 @@ int app_server_stop(int argc,char **argv,struct command_line_option *o) int app_server_status(int argc,char **argv,struct command_line_option *o) { - char *instancepath - =cli_arg(argc,argv,o,"instance path",getenv("SERVALINSTANCE_PATH")); - if (!instancepath) instancepath=DEFAULT_INSTANCE_PATH; - - char filename[1024]; - char line[1024]; - FILE *f; + thisinstancepath = cli_arg(argc, argv, o, "instance path", NULL); /* Display configuration information */ - snprintf(filename,1023,"%s/serval.conf",instancepath); filename[1023]=0; - f=fopen(filename,"r"); - if (f) { + char filename[1024]; + FILE *f; + if (FORM_SERVAL_INSTANCE_PATH(filename, "serval.conf") && (f = fopen(filename, "r"))) { + char line[1024]; line[0]=0; fgets(line,1024,f); printf("\nServal Mesh configuration:\n"); while(line[0]) { @@ -342,12 +334,13 @@ int app_server_status(int argc,char **argv,struct command_line_option *o) } fclose(f); } + /* Display running status of daemon from serval.pid file */ int pid=-1; - int running = servalNodeRunning(&pid,instancepath); + int running = servalNodeRunning(&pid); if (running<0) return -1; - printf("For Serval Mesh instance %s:\n",instancepath); + printf("For Serval Mesh instance %s:\n", serval_instancepath()); if (running) printf(" Serval mesh process is running (pid=%d)\n",pid); else @@ -359,7 +352,6 @@ int app_server_status(int argc,char **argv,struct command_line_option *o) int app_mdp_ping(int argc,char **argv,struct command_line_option *o) { char *sid=cli_arg(argc,argv,o,"SID|broadcast","broadcast"); - char *instancepath=serval_instancepath(); /* MDP frames consist of: destination SID (32 bytes) @@ -507,25 +499,26 @@ int app_server_set(int argc,char **argv,struct command_line_option *o) char *val=cli_arg(argc,argv,o,"value",""); char conffile[1024]; - char tempfile[1024]; - - snprintf(conffile,1024,"%s/serval.conf",serval_instancepath()); - snprintf(tempfile,1024,"%s/serval.conf.temp",serval_instancepath()); - - FILE *in=fopen(conffile,"r"); - if (!in) in=fopen(conffile,"w"); - if (!in) + FILE *in; + if (!FORM_SERVAL_INSTANCE_PATH(conffile, "serval.conf") || + !((in = fopen(conffile, "r")) || (in = fopen(conffile, "w"))) + ) { return WHY("could not read configuration file."); - FILE *out=fopen(tempfile,"w"); - if (!out) { + } + + char tempfile[1024]; + FILE *out; + if (!FORM_SERVAL_INSTANCE_PATH(tempfile, "serval.conf.temp") || + !(out = fopen(tempfile, "w")) + ) { fclose(in); return WHY("could not write temporary file."); } - char line[1024]; /* Read and write lines of config file, replacing the variable in question if required. If the variable didn't already exist, then write it out at the end. */ + char line[1024]; int found=0; int varlen=strlen(var); line[0]=0; fgets(line,1024,in); @@ -543,7 +536,7 @@ int app_server_set(int argc,char **argv,struct command_line_option *o) fclose(in); fclose(out); if (rename(tempfile,conffile)) { - return WHY("Failed to put temporary config file into place."); + return WHYF("Failed to rename \"%s\" to \"%s\".", tempfile, conffile); } return 0; diff --git a/dna.c b/dna.c index bf5686a5..aef81763 100644 --- a/dna.c +++ b/dna.c @@ -295,19 +295,29 @@ char *serval_instancepath() return instancepath; } +int form_serval_instance_path(char *buf, size_t bufsiz, const char *path) +{ + if (snprintf(buf, bufsiz, "%s/%s", serval_instancepath(), path) < bufsiz) + return 1; + fprintf(stderr, "Cannot form pathname \"%s/%s\" -- buffer too small (%lu bytes)", serval_instancepath(), path, (unsigned long)bufsiz); + return 0; +} + void servalShutdownCleanly() { WHY("Shutting down as requested."); /* Try to remove shutdown and PID files and exit */ - char *instancepath=serval_instancepath(); char filename[1024]; - snprintf(filename,1024,"%s/doshutdown",instancepath); - unlink(filename); - snprintf(filename,1024,"%s/serval.pid",instancepath); - unlink(filename); - if (mdp_client_socket==-1) { - snprintf(filename,1024,"%s/mdp.socket",instancepath); + if (FORM_SERVAL_INSTANCE_PATH(filename, "doshutdown")) { unlink(filename); + } + if (FORM_SERVAL_INSTANCE_PATH(filename, "serval.pid")) { + unlink(filename); + } + if (mdp_client_socket==-1) { + if (FORM_SERVAL_INSTANCE_PATH(filename, "mdp.socket")) { + unlink(filename); + } } else { overlay_mdp_client_done(); } diff --git a/overlay_interface.c b/overlay_interface.c index 134158fd..a1efe2df 100644 --- a/overlay_interface.c +++ b/overlay_interface.c @@ -250,10 +250,9 @@ int overlay_interface_init(char *name,struct sockaddr_in src_addr,struct sockadd if (name[0]=='>') { I(fileP)=1; char dummyfile[1024]; - snprintf(dummyfile,1024,"%s/%s",serval_instancepath(),&name[1]); - I(fd) = open(dummyfile,O_APPEND|O_RDWR); - if (I(fd)<1) + if (!FORM_SERVAL_INSTANCE_PATH(dummyfile, &name[1]) || (I(fd) = open(dummyfile,O_APPEND|O_RDWR)) < 1) { return WHY("could not open dummy interface file for append"); + } /* Seek to end of file as initial reading point */ I(offset)=lseek(I(fd),0,SEEK_END); /* socket gets reused to hold file offset */ /* XXX later add pretend location information so that we can decide which "packets" to receive diff --git a/overlay_mdp.c b/overlay_mdp.c index c8c8dc4e..efac345b 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -59,9 +59,9 @@ int overlay_mdp_setup_sockets() } #endif if (mdp_named_socket==-1) { - char *instancepath=serval_instancepath(); - if (strlen(instancepath)>85) return WHY("Instance path too long to allow construction of named unix domain socket."); - snprintf(&name.sun_path[0],100,"%s/mdp.socket",instancepath); + if (!form_serval_instance_path(&name.sun_path[0], 100, "mdp.socket")) { + return WHY("Cannot construct name of unix domain socket."); + } unlink(&name.sun_path[0]); len = 0+strlen(&name.sun_path[0]) + sizeof(name.sun_family)+1; mdp_named_socket = socket(AF_UNIX, SOCK_DGRAM, 0); @@ -701,15 +701,10 @@ int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms) if (len<0) return WHY("MDP frame invalid (could not compute length)"); /* Construct name of socket to send to. */ - char mdp_socket_name[101]; - mdp_socket_name[100]=0; - snprintf(mdp_socket_name,100,"%s/mdp.socket",serval_instancepath()); - if (mdp_socket_name[100]) { - return WHY("instance path is too long (unix domain named sockets have a short maximum path length)"); - } struct sockaddr_un name; name.sun_family = AF_UNIX; - strcpy(name.sun_path, mdp_socket_name); + if (!FORM_SERVAL_INSTANCE_PATH(name.sun_path, "mdp.socket")) + return -1; int result=sendto(mdp_client_socket, mdp, len, 0, (struct sockaddr *)&name, sizeof(struct sockaddr_un)); @@ -756,8 +751,6 @@ int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms) int overlay_mdp_client_init() { - char mdp_temporary_socket[1024]; - mdp_temporary_socket[0]=0; if (mdp_client_socket==-1) { /* Open socket to MDP server (thus connection is always local) */ WHY("Use of abstract name space socket for Linux not implemented"); @@ -769,12 +762,12 @@ int overlay_mdp_client_init() } /* We must bind to a temporary file name */ - snprintf(mdp_temporary_socket,1024,"%s/mdp-client.socket",serval_instancepath()); - unlink(mdp_temporary_socket); struct sockaddr_un name; name.sun_family = AF_UNIX; - snprintf(&name.sun_path[0],100,"%s",mdp_temporary_socket); - int len = 1+strlen(&name.sun_path[0]) + sizeof(name.sun_family)+1; + if (!FORM_SERVAL_INSTANCE_PATH(name.sun_path, "mdb-client.socket")) + return WHY("Could not form MDP client socket name"); + unlink(name.sun_path); + int len = 1 + strlen(name.sun_path) + sizeof(name.sun_family) + 1; int r=bind(mdp_client_socket, (struct sockaddr *)&name, len); if (r) { WHY("Could not bind MDP client socket to file name"); @@ -789,9 +782,10 @@ int overlay_mdp_client_init() int overlay_mdp_client_done() { char mdp_temporary_socket[1024]; - snprintf(mdp_temporary_socket,1024,"%s/mdp-client.socket",serval_instancepath()); - unlink(mdp_temporary_socket); - if (mdp_client_socket!=-1) close(mdp_client_socket); + if (FORM_SERVAL_INSTANCE_PATH(mdp_temporary_socket, "mdp-client.socket")) + unlink(mdp_temporary_socket); + if (mdp_client_socket!=-1) + close(mdp_client_socket); mdp_client_socket=-1; return 0; } @@ -809,12 +803,10 @@ int overlay_mdp_client_poll(long long timeout_ms) int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl) { char mdp_socket_name[101]; - mdp_socket_name[100]=0; - snprintf(mdp_socket_name,100,"%s/mdp.socket",serval_instancepath()); - + if (!FORM_SERVAL_INSTANCE_PATH(mdp_socket_name, "mdp.socket")) + return -1; /* Check if reply available */ - fcntl(mdp_client_socket, F_SETFL, - fcntl(mdp_client_socket, F_GETFL, NULL)|O_NONBLOCK); + fcntl(mdp_client_socket, F_SETFL, fcntl(mdp_client_socket, F_GETFL, NULL)|O_NONBLOCK); unsigned char recvaddrbuffer[1024]; struct sockaddr *recvaddr=(struct sockaddr *)recvaddrbuffer; unsigned int recvaddrlen=sizeof(recvaddrbuffer); diff --git a/rhizome_bundle.c b/rhizome_bundle.c index db41728d..3ed93280 100644 --- a/rhizome_bundle.c +++ b/rhizome_bundle.c @@ -63,6 +63,11 @@ rhizome_manifest *rhizome_read_manifest_file(char *filename,int bufferP,int flag line[i]=0; /* Ignore blank lines */ if (line[0]==0) continue; + /* Ignore comment lines */ + if (line[0] == '#' || line[0] == '!') continue; + /* Parse property lines */ + /* This could be improved to parse Java's Properties.store() output, by handling backlash + escapes and continuation lines */ if (sscanf(line,"%[^=]=%[^\n\r]",var,value)==2) { if (rhizome_manifest_get(m,var,NULL,0)!=NULL) { diff --git a/serval.h b/serval.h index c186d78b..358b2309 100644 --- a/serval.h +++ b/serval.h @@ -650,6 +650,7 @@ extern overlay_txqueue overlay_tx[OQ_MAX]; int setReason(char *fmt, ...); #define WHY(X) setReason("%s:%d:%s() %s",__FILE__,__LINE__,__FUNCTION__,X) +#define WHYF(F, ...) setReason("%s:%d:%s() " F, __FILE__, __LINE__, __FUNCTION__, ##__VA_ARGS__) overlay_buffer *ob_new(int size); int ob_free(overlay_buffer *b); @@ -985,6 +986,12 @@ int _memabuseCheck(const char *func,const char *file,const int line); char *thisinstancepath; char *serval_instancepath(); +int form_serval_instance_path(char * buf, size_t bufsiz, const char *path); + +/* Handy statement for forming a path to an instance file in a char buffer whose declaration + * is in scope (so that sizeof(buf) will work). Evaluates to true if the pathname fitted into + * the provided buffer, false (0) otherwise (after printing a message to stderr). */ +#define FORM_SERVAL_INSTANCE_PATH(buf, path) (form_serval_instance_path(buf, sizeof(buf), (path))) int overlay_mdp_get_fds(struct pollfd *fds,int *fdcount,int fdmax); int overlay_mdp_poll(); diff --git a/server.c b/server.c index 1d8a69a1..6f351974 100644 --- a/server.c +++ b/server.c @@ -125,12 +125,12 @@ int server(char *backing_file,int size,int foregroundMode) /* Record PID */ char filename[1024]; - char *instancepath=getenv("SERVALINSTANCE_PATH"); - if (!instancepath) instancepath=DEFAULT_INSTANCE_PATH; - snprintf(filename,1023,"%s/serval.pid",instancepath); filename[1023]=0; + if (!FORM_SERVAL_INSTANCE_PATH(filename, "serval.pid")) + exit(-1); FILE *f=fopen(filename,"w"); if (!f) { WHY("Could not write to PID file"); + perror("fopen"); exit(-1); } fprintf(f,"%d\n",getpid());