On 2015-12-01 at 23:04:18 +0100, Vadim Kochan <[email protected]> wrote: > Add -D option for bpfc & trafgen to make possible pass -Dkey=value > to C preprocessor. Characters like .?*()[]-^|`!#$&<>;\ are quoted > to do not trick with shell's special symbols (e.g. process spawning), > \ is ignored only if it goes before " - to make possible pass escaped > text for trafgen's conf file.
Please split this up into at least 4 patches to make review easier: 1) string list helper 1) helpers to create/check/... argument list (cpp module) 2) trafgen integration 3) bpfc integration I'm also still not a big fan of the shell escaping solution. This adds a lot of string handling code which needs to be reviewed _very_ carefully. If somehow possible I'd like to avoid this alltogether. Could we maybe get rid of the call to system() alltogether and instead use fork() + execvp() + wait() and pass the definitions args as we got them from getopt? You'd need to use the "-o" option to cpp instead of the input redirection, but that should be fine... A few minor remarks below, but not a complete review (due to limited time at the moment). > Signed-off-by: Vadim Kochan <[email protected]> > --- > bpf_parser.y | 6 ++-- > bpfc.8 | 6 ++++ > bpfc.c | 16 ++++++++-- > cpp.c | 97 > +++++++++++++++++++++++++++++++++++++++++++++++++++++--- > cpp.h | 2 +- > str.c | 10 ++++++ > str.h | 1 + > trafgen.8 | 7 ++++ > trafgen.c | 22 +++++++++---- > trafgen_conf.h | 3 +- > trafgen_parser.y | 5 +-- > 11 files changed, 154 insertions(+), 21 deletions(-) > > diff --git a/bpf_parser.y b/bpf_parser.y > index 003c031..28e8d11 100644 > --- a/bpf_parser.y > +++ b/bpf_parser.y > @@ -28,7 +28,7 @@ > #include "cpp.h" > > int compile_filter(char *file, int verbose, int bypass, int format, > - bool invoke_cpp); > + bool invoke_cpp, char **cpp_defs); c > > static int curr_instr = 0; > > @@ -735,7 +735,7 @@ static void pretty_printer(const struct sock_fprog *prog, > int format) > } > > int compile_filter(char *file, int verbose, int bypass, int format, > - bool invoke_cpp) > + bool invoke_cpp, char **cpp_defs) > { > int i; > struct sock_fprog res; > @@ -745,7 +745,7 @@ int compile_filter(char *file, int verbose, int bypass, > int format, > memset(tmp_file, 0, sizeof(tmp_file)); > > if (invoke_cpp) { > - ret = cpp_exec(file, tmp_file, sizeof(tmp_file)); > + ret = cpp_exec(file, tmp_file, sizeof(tmp_file), cpp_defs); > if (ret) { > fprintf(stderr, "Failed to invoke C preprocessor!\n"); > goto exit; > diff --git a/bpfc.8 b/bpfc.8 > index 8a99e2e..eeb8356 100644 > --- a/bpfc.8 > +++ b/bpfc.8 > @@ -65,6 +65,12 @@ Pass the bpf program through the C preprocessor before > reading it in > bpfc. This allows #define and #include directives (e.g. to include > definitions from system headers) to be used in the bpf program. > .PP > +.SS -D, --define > +Add macro/define for C preprocessor to use it within bpf file. You may need > the shell's > +quoting syntax to protect characters such as spaces that have a meaning in > the shell syntax. > +By default the following characters .?*()[]-^|`!$&<>;\\ are escaped > automatically when > +calling C preprocessor. Please explicitly mention that this flag is only useful together with -p/--cpp, same goes for the trafgen manpage. > +.PP > .SS -f <format>, --format <format> > Specify a different output format than the default that is netsniff-ng > compatible. The <format> specifier can be: C, netsniff-ng, xt_bpf, tcpdump. > diff --git a/bpfc.c b/bpfc.c > index d360cf5..fb7a9e5 100644 > --- a/bpfc.c > +++ b/bpfc.c > @@ -17,12 +17,14 @@ > #include "die.h" > #include "bpf.h" > #include "config.h" > +#include "str.h" > > -static const char *short_options = "vhi:Vdbf:p"; > +static const char *short_options = "vhi:Vdbf:pD:"; > static const struct option long_options[] = { > {"input", required_argument, NULL, 'i'}, > {"format", required_argument, NULL, 'f'}, > {"cpp", no_argument, NULL, 'p'}, > + {"define", required_argument, NULL, 'D'}, > {"verbose", no_argument, NULL, 'V'}, > {"bypass", no_argument, NULL, 'b'}, > {"dump", no_argument, NULL, 'd'}, > @@ -39,7 +41,7 @@ static const char *copyright = "Please report bugs to > <netsniff-ng@googlegroups. > "There is NO WARRANTY, to the extent permitted by law."; > > extern int compile_filter(char *file, int verbose, int bypass, int format, > - bool invoke_cpp); > + bool invoke_cpp, char **cpp_defs); > > static void __noreturn help(void) > { > @@ -49,6 +51,7 @@ static void __noreturn help(void) > "Options:\n" > " -i|--input <program/-> Berkeley Packet Filter file/stdin\n" > " -p|--cpp Run bpf program through C > preprocessor\n" > + " -D|--define Add macro/define for C preprocessor\n" > " -f|--format <format> Output format: > C|netsniff-ng|xt_bpf|tcpdump\n" > " -b|--bypass Bypass filter validation (e.g. for bug > testing)\n" > " -V|--verbose Be more verbose\n" > @@ -81,6 +84,8 @@ int main(int argc, char **argv) > { > int ret, verbose = 0, c, opt_index, bypass = 0, format = 0; > bool invoke_cpp = false; > + char **cpp_defs = NULL; > + int defs_count = 0; > char *file = NULL; > > setfsuid(getuid()); > @@ -104,6 +109,9 @@ int main(int argc, char **argv) > case 'p': > invoke_cpp = true; > break; > + case 'D': > + cpp_defs = str_list_insert(cpp_defs, &defs_count, > optarg); optarg should be xstrdup()'ed here (or inside str_list_insert) to be safe against changes of argv/optarg. Same for trafgen.c > + break; > case 'f': > if (!strncmp(optarg, "C", 1) || > !strncmp(optarg, "netsniff-ng", 11)) > @@ -146,8 +154,10 @@ int main(int argc, char **argv) > if (!file) > panic("No Berkeley Packet Filter program specified!\n"); > > - ret = compile_filter(file, verbose, bypass, format, invoke_cpp); > + ret = compile_filter(file, verbose, bypass, format, invoke_cpp, > cpp_defs); > > + if (cpp_defs) > + xfree(cpp_defs); > xfree(file); > return ret; > } > diff --git a/cpp.c b/cpp.c > index 6734eac..1cee8b8 100644 > --- a/cpp.c > +++ b/cpp.c > @@ -4,21 +4,108 @@ > #include "str.h" > #include "xmalloc.h" > > -int cpp_exec(char *in_file, char *out_file, size_t out_len) > +static bool is_special_char(char ch) > { > + char *chars = ".?*()[]-^|`!$&<>;"; > + > + for (; *chars; chars++) { > + if (*chars == ch) > + return true; > + } > + > + return false; > +} > + > +size_t strlen_escaped(char *s) This function should be static (if present for v3 at all). > +{ > + size_t len = 0; > + > + for (; *s; s++) { > + if (*s == '\\' && *(s + 1) != '"') > + len++; > + else if (is_special_char(*s)) > + len++; > + > + len++; > + } > + > + return len; > +} > + > +static char *cpp_defs2args(char **cpp_defs, int *len) > +{ > + char *args = NULL; > + int defs_len = 0; > + int i; > + > + if (!cpp_defs || !*cpp_defs) > + return NULL; > + > + for (i = 0; cpp_defs[i]; i++) > + defs_len += strlen_escaped(cpp_defs[i]) + 3; /* "-D" + space */ > + > + args = xmalloc(defs_len * sizeof(char)); > + > + for (i = 0; *cpp_defs; cpp_defs++) { > + char *def = *cpp_defs; > + > + args[i++] = ' '; > + args[i++] = '-'; > + args[i++] = 'D'; > + > + for (; *def; def++) { > + if (*def == '\\' && *(def + 1) != '"') > + args[i++] = '\\'; > + else if (is_special_char(*def)) > + args[i++] = '\\'; > + > + args[i++] = *def; > + } > + } > + > + *len = defs_len; > + args[i] = '\0'; > + return args; > +} > + > +int cpp_exec(char *in_file, char *out_file, size_t out_len, char **cpp_defs) > +{ > + char *cmd_fmt = "cpp -I" ETCDIRE_STRING " %s %s > %s"; > char *tmp = xstrdup(in_file); > - char cmd[256], *base; > + int tmp_len; > + char *defs_args = NULL; > + int defs_len = 0; > + char *args; > + char *cmd = NULL; > + int cmd_len; > + char *base; > int ret = 0; > > base = basename(tmp); > > - slprintf(out_file, out_len, "/tmp/.tmp-%u-%s", rand(), base); > - slprintf(cmd, sizeof(cmd), "cpp -I" ETCDIRE_STRING " %s > %s", > - in_file, out_file); > + if (cpp_defs && *cpp_defs) > + defs_args = cpp_defs2args(cpp_defs, &defs_len); > + > + tmp_len = slprintf(out_file, out_len, "/tmp/.tmp-%u-%s", rand(), base); > + > + cmd_len = tmp_len + defs_len + strlen(in_file) + strlen(cmd_fmt); > + cmd = xmalloc(cmd_len * sizeof(char)); > + > + args = defs_args ? defs_args : ""; > + > + if ((slprintf(cmd, cmd_len, cmd_fmt, args, in_file, out_file)) < 0) { > + fprintf(stderr, "Could not build command line for cpp\n"); > + ret = -1; > + goto exit; > + } > > if (system(cmd) != 0) > ret = -1; > > +exit: > + if (defs_args) > + xfree(defs_args); > xfree(tmp); > + xfree(cmd); > return ret; > } > diff --git a/cpp.h b/cpp.h > index a3e295d..f834a56 100644 > --- a/cpp.h > +++ b/cpp.h > @@ -1,6 +1,6 @@ > #ifndef CPP_H > #define CPP_H > > -extern int cpp_exec(char *in_file, char *out_file, size_t out_len); > +extern int cpp_exec(char *in_file, char *out_file, size_t out_len, char > **cpp_defs); > > #endif > diff --git a/str.c b/str.c > index f4cb099..6a1d82a 100644 > --- a/str.c > +++ b/str.c > @@ -109,3 +109,13 @@ char *argv2str(int startind, int argc, char **argv) > > return str; > } > + > +char **str_list_insert(char **str_list, int *count, char *str) > +{ > + str_list = (char **)xrealloc(str_list, (*count + 2) * sizeof(char *)); > + str_list[*count] = str; > + str_list[*count + 1] = NULL; > + > + *count += 1; > + return str_list; > +} > diff --git a/str.h b/str.h > index 7d078da..e68b2cc 100644 > --- a/str.h > +++ b/str.h > @@ -9,5 +9,6 @@ extern int slprintf_nocheck(char *dst, size_t size, const > char *fmt, ...); > extern char *strtrim_right(char *p, char c); > extern noinline void *xmemset(void *s, int c, size_t n); > extern char *argv2str(int startind, int argc, char **argv); > +extern char **str_list_insert(char **str_list, int *count, char *str); > > #endif /* STR_H */ > diff --git a/trafgen.8 b/trafgen.8 > index 18b2b61..ec9a28e 100644 > --- a/trafgen.8 > +++ b/trafgen.8 > @@ -79,6 +79,13 @@ Pass the packet configuration to the C preprocessor before > reading it into > trafgen. This allows #define and #include directives (e.g. to include > definitions from system headers) to be used in the trafgen configuration > file. > .PP > +.SS -D, --define > +Add macro/define for C preprocessor to use it within trafgen's file. You may > need the shell's > +quoting syntax to protect characters such as spaces that have a meaning in > the shell syntax. > +By default the following characters .?*()[]-^|`!$&<>;\\ are escaped > automatically when > +calling C preprocessor, \\ is ignored only if it goes before " symbol which > is needed > +to make possible pass escaped text. > +.PP > .SS -J, --jumbo-support > By default trafgen's ring buffer frames are of a fixed size of 2048 bytes. > This means that if you're expecting jumbo frames or even super jumbo frames > to > diff --git a/trafgen.c b/trafgen.c > index d01f160..73e97ad 100644 > --- a/trafgen.c > +++ b/trafgen.c > @@ -65,6 +65,8 @@ struct ctx { > struct timespec gap; > struct sockaddr_in dest; > char *packet_str; > + bool invoke_cpp; > + char **cpp_defs; Moving invoke_cpp to struct ctx should be a separate preparatory patch. > }; > > struct cpu_stats { > @@ -83,7 +85,7 @@ size_t plen = 0; > struct packet_dyn *packet_dyn = NULL; > size_t dlen = 0; > > -static const char *short_options = "d:c:n:t:vJhS:rk:i:o:VRs:P:eE:pu:g:CHQq"; > +static const char *short_options = > "d:c:n:t:vJhS:rk:i:o:VRs:P:eE:pu:g:CHQqD:"; > static const struct option long_options[] = { > {"dev", required_argument, NULL, 'd'}, > {"out", required_argument, NULL, 'o'}, > @@ -105,6 +107,7 @@ static const struct option long_options[] = { > {"jumbo-support", no_argument, NULL, 'J'}, > {"no-cpu-stats", no_argument, NULL, 'C'}, > {"cpp", no_argument, NULL, 'p'}, > + {"define", required_argument, NULL, 'D'}, > {"rfraw", no_argument, NULL, 'R'}, > {"rand", no_argument, NULL, 'r'}, > {"verbose", no_argument, NULL, 'V'}, > @@ -163,6 +166,7 @@ static void __noreturn help(void) > " -i|-c|--in|--conf <cfg/-> Packet configuration > file/stdin\n" > " -o|-d|--out|--dev <netdev> Networking device i.e., eth0\n" > " -p|--cpp Run packet config through C > preprocessor\n" > + " -D|--define Add macro/define for C > preprocessor\n" > " -J|--jumbo-support Support 64KB super jumbo frames > (def: 2048B)\n" > " -R|--rfraw Inject raw 802.11 frames\n" > " -s|--smoke-test <ipv4> Probe if machine survived > fuzz-tested packet\n" > @@ -823,12 +827,12 @@ static void xmit_packet_precheck(struct ctx *ctx, > unsigned int cpu) > } > > static void main_loop(struct ctx *ctx, char *confname, bool slow, > - unsigned int cpu, bool invoke_cpp, unsigned long orig_num) > + unsigned int cpu, unsigned long orig_num) > { > if (ctx->packet_str) > compile_packets_str(ctx->packet_str, ctx->verbose, cpu); > else > - compile_packets(confname, ctx->verbose, cpu, invoke_cpp); > + compile_packets(confname, ctx->verbose, cpu, ctx->invoke_cpp, > ctx->cpp_defs); > > preprocess_packets(); > > @@ -886,7 +890,7 @@ static void on_panic_del_rfmon(void *arg) > > int main(int argc, char **argv) > { > - bool slow = false, invoke_cpp = false, reseed = true, cpustats = true; > + bool slow = false, reseed = true, cpustats = true; > bool prio_high = false, set_irq_aff = true, set_sock_mem = true; > int c, opt_index, vals[4] = {0}, irq; > uint64_t gap = 0; > @@ -896,6 +900,7 @@ int main(int argc, char **argv) > unsigned long long tx_packets, tx_bytes; > struct ctx ctx; > int min_opts = 5; > + int defs_count = 0; > > fmemset(&ctx, 0, sizeof(ctx)); > ctx.cpus = get_number_cpus_online(); > @@ -922,7 +927,10 @@ int main(int argc, char **argv) > example(); > break; > case 'p': > - invoke_cpp = true; > + ctx.invoke_cpp = true; > + break; > + case 'D': > + ctx.cpp_defs = str_list_insert(ctx.cpp_defs, > &defs_count, optarg); > break; > case 'V': > ctx.verbose = true; > @@ -1133,7 +1141,7 @@ int main(int argc, char **argv) > srand(seed); > > cpu_affinity(i); > - main_loop(&ctx, confname, slow, i, invoke_cpp, > orig_num); > + main_loop(&ctx, confname, slow, i, orig_num); > > goto thread_out; > case -1: > @@ -1184,6 +1192,8 @@ thread_out: > free(ctx.rhost); > free(confname); > free(ctx.packet_str); > + if (ctx.cpp_defs) > + free(ctx.cpp_defs); > > return 0; > } > diff --git a/trafgen_conf.h b/trafgen_conf.h > index deadb7c..24092bb 100644 > --- a/trafgen_conf.h > +++ b/trafgen_conf.h > @@ -56,7 +56,8 @@ static inline bool packet_dyn_has_only_csums(struct > packet_dyn *p) > } > > extern void compile_packets_str(char *str, bool verbose, unsigned int cpu); > -extern void compile_packets(char *file, bool verbose, unsigned int cpu, bool > invoke_cpp); > +extern void compile_packets(char *file, bool verbose, unsigned int cpu, > + bool invoke_cpp, char **cpp_defs); > extern void cleanup_packets(void); > > #endif /* TRAFGEN_CONF */ > diff --git a/trafgen_parser.y b/trafgen_parser.y > index 44751ab..3f7bfa7 100644 > --- a/trafgen_parser.y > +++ b/trafgen_parser.y > @@ -590,7 +590,8 @@ void cleanup_packets(void) > free(packet_dyn); > } > > -void compile_packets(char *file, bool verbose, unsigned int cpu, bool > invoke_cpp) > +void compile_packets(char *file, bool verbose, unsigned int cpu, bool > invoke_cpp, > + char **cpp_defs) > { > char tmp_file[128]; > int ret = -1; > @@ -599,7 +600,7 @@ void compile_packets(char *file, bool verbose, unsigned > int cpu, bool invoke_cpp > our_cpu = cpu; > > if (invoke_cpp) { > - if (cpp_exec(file, tmp_file, sizeof(tmp_file))) { > + if (cpp_exec(file, tmp_file, sizeof(tmp_file), cpp_defs)) { > fprintf(stderr, "Failed to invoke C preprocessor!\n"); > goto err; > } > -- > 2.6.2 > -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
