This patch enhances implementation of getopt() and getopt_long() functions to work with PIEs.
Newer GCC compiler optimizes PIEs by emitting machine code with so called copy relocations and affects how caller (PIEs) and callee (OSv kernel) see same logical global variables like optarg. New implementation of getopt() and getopt_long() detects if caller uses its (other) copies of global variables like optind and copies its value to the kernel copy. Likewise on return from the getopt() function it copies values of all "output" global variables from kernel to the equivalent caller copies of those variables. Fixes #689 Signed-off-by: Waldemar Kozaczuk <[email protected]> --- core/elf.cc | 14 +++ include/osv/elf.hh | 4 + libc/misc/getopt.cc | 62 +++++++++-- libc/misc/getopt_long.cc | 37 ++++++- modules/tests/Makefile | 9 +- tests/tst-getopt.cc | 232 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 345 insertions(+), 13 deletions(-) create mode 100644 tests/tst-getopt.cc diff --git a/core/elf.cc b/core/elf.cc index ca9226f6..84e6fca3 100644 --- a/core/elf.cc +++ b/core/elf.cc @@ -147,6 +147,20 @@ void* object::lookup(const char* symbol) return sm.relocated_addr(); } +void* object::cached_lookup(const char* name) +{ + auto name_str = std::string(name); + auto cached_address_it = _cached_symbols.find(name_str); + if (cached_address_it != _cached_symbols.end()) { + return cached_address_it->second; + } + else { + void *symbol_address = lookup(name); + _cached_symbols[name_str] = symbol_address; + return symbol_address; + } +} + std::vector<Elf64_Shdr> object::sections() { size_t bytes = size_t(_ehdr.e_shentsize) * _ehdr.e_shnum; diff --git a/include/osv/elf.hh b/include/osv/elf.hh index 0f1792c7..86ac1cf6 100644 --- a/include/osv/elf.hh +++ b/include/osv/elf.hh @@ -14,6 +14,7 @@ #include <stack> #include <memory> #include <unordered_set> +#include <unordered_map> #include <osv/types.h> #include <atomic> @@ -349,6 +350,7 @@ public: void run_fini_funcs(); template <typename T = void> T* lookup(const char* name); + void* cached_lookup(const char* name); dladdr_info lookup_addr(const void* addr); bool contains_addr(const void* addr); ulong module_index() const; @@ -415,6 +417,8 @@ protected: bool _is_executable; bool is_core(); + std::unordered_map<std::string,void*> _cached_symbols; + // Keep list of references to other modules, to prevent them from being // unloaded. When this object is unloaded, the reference count of all // objects listed here goes down, and they too may be unloaded. diff --git a/libc/misc/getopt.cc b/libc/misc/getopt.cc index 70a11414..a9a749ec 100644 --- a/libc/misc/getopt.cc +++ b/libc/misc/getopt.cc @@ -4,6 +4,8 @@ #include <limits.h> #include <stdlib.h> #include <libc/libc.hh> +#include <osv/sched.hh> +#include <osv/app.hh> extern "C" { @@ -13,6 +15,30 @@ int optind=1, opterr=1, optopt, __optpos, __optreset=0; #define optpos __optpos weak_alias(__optreset, optreset); +#define COPY_GLOBALS_AND_RETURN(retval) { \ + if (other_optarg) { \ + *other_optarg = optarg; \ + } \ + if (other_optind) { \ + *other_optind = optind; \ + }\ + if (other_optopt) { \ + *other_optopt = optopt; \ + }\ + return retval; \ +} + +// As explained in http://www.shrubbery.net/solaris9ab/SUNWdev/LLM/p22.html#CHAPTER4-84604 +// newer versions of gcc produce position independent executables with copies of +// some global variables like those used by getopt() and getopt_long() for optimizations reason. +// In those circumstances the caller of these functions uses different copies of +// global variables (like optind) than the getopt() code that is part of OSv kernel. +// For that reason in the beginning of the function we need to copy values of the caller +// copies of those variables to the kernel placeholders. Likewise on every from the function +// we need to copy the values of kernel copies of global variables to the caller ones. +// +// See http://man7.org/linux/man-pages/man3/getopt.3.html +// int getopt(int argc, char * const argv[], const char *optstring) { int i; @@ -20,6 +46,26 @@ int getopt(int argc, char * const argv[], const char *optstring) int k, l; char *optchar; + char **other_optarg = nullptr; + int* other_optind = nullptr, *other_optopt = nullptr, *other_opterr = nullptr; + + auto __runtime = sched::thread::current()->app_runtime(); + if (__runtime) { + auto obj = __runtime->app.lib(); + other_optarg = reinterpret_cast<char**>(obj->cached_lookup("optarg")); + other_optind = reinterpret_cast<int*>(obj->cached_lookup("optind")); + other_optopt = reinterpret_cast<int*>(obj->cached_lookup("optopt")); + other_opterr = reinterpret_cast<int*>(obj->cached_lookup("opterr")); + } + + // Copy values of caller copies + if (other_opterr) { + opterr = *other_opterr; + } + if (other_optind) { + optind = *other_optind; + } + if (!optind || __optreset) { __optreset = 0; __optpos = 0; @@ -27,9 +73,11 @@ int getopt(int argc, char * const argv[], const char *optstring) } if (optind >= argc || !argv[optind] || argv[optind][0] != '-' || !argv[optind][1]) - return -1; - if (argv[optind][1] == '-' && !argv[optind][2]) - return optind++, -1; + COPY_GLOBALS_AND_RETURN(-1) + if (argv[optind][1] == '-' && !argv[optind][2]) { + optind++; + COPY_GLOBALS_AND_RETURN(-1) + } if (!optpos) optpos++; if ((k = mbtowc(&c, argv[optind]+optpos, MB_LEN_MAX)) < 0) { @@ -54,23 +102,23 @@ int getopt(int argc, char * const argv[], const char *optstring) write(2, optchar, k); write(2, "\n", 1); } - return '?'; + COPY_GLOBALS_AND_RETURN('?') } if (optstring[i+1] == ':') { if (optind >= argc) { - if (optstring[0] == ':') return ':'; + if (optstring[0] == ':') COPY_GLOBALS_AND_RETURN(':') if (opterr) { write(2, argv[0], strlen(argv[0])); write(2, ": option requires an argument: ", 31); write(2, optchar, k); write(2, "\n", 1); } - return '?'; + COPY_GLOBALS_AND_RETURN('?') } optarg = argv[optind++] + optpos; optpos = 0; } - return c; + COPY_GLOBALS_AND_RETURN(c) } weak_alias(getopt, __posix_getopt); diff --git a/libc/misc/getopt_long.cc b/libc/misc/getopt_long.cc index fbde89fc..184d22fe 100644 --- a/libc/misc/getopt_long.cc +++ b/libc/misc/getopt_long.cc @@ -1,6 +1,18 @@ #include <stddef.h> #include <getopt.h> #include <stdio.h> +#include <osv/sched.hh> +#include <osv/app.hh> + +#define COPY_GLOBALS_AND_RETURN(retval) { \ + if (other_optarg) { \ + *other_optarg = optarg; \ + } \ + if (other_optind) { \ + *other_optind = optind; \ + } \ + return retval; \ +} extern "C" { @@ -8,12 +20,27 @@ extern int __optpos, __optreset; static int __getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx, int longonly) { + char **other_optarg = nullptr; + int* other_optind = nullptr; + + auto __runtime = sched::thread::current()->app_runtime(); + if (__runtime) { + auto obj = __runtime->app.lib(); + other_optarg = reinterpret_cast<char**>(obj->cached_lookup("optarg")); + other_optind = reinterpret_cast<int*>(obj->cached_lookup("optind")); + } + + // Copy value of caller copy + if (other_optind) { + optind = *other_optind; + } + if (!optind || __optreset) { __optreset = 0; __optpos = 0; optind = 1; } - if (optind >= argc || !argv[optind] || argv[optind][0] != '-') return -1; + if (optind >= argc || !argv[optind] || argv[optind][0] != '-') COPY_GLOBALS_AND_RETURN(-1) if ((longonly && argv[optind][1]) || (argv[optind][1] == '-' && argv[optind][2])) { @@ -30,20 +57,20 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con } else { if (longopts[i].has_arg == required_argument) { if (!(optarg = argv[++optind])) - return ':'; + COPY_GLOBALS_AND_RETURN(':') } else optarg = NULL; } optind++; if (idx) *idx = i; if (longopts[i].flag) { *longopts[i].flag = longopts[i].val; - return 0; + COPY_GLOBALS_AND_RETURN(0) } - return longopts[i].val; + COPY_GLOBALS_AND_RETURN(longopts[i].val) } if (argv[optind][1] == '-') { optind++; - return '?'; + COPY_GLOBALS_AND_RETURN('?') } } return getopt(argc, argv, optstring); diff --git a/modules/tests/Makefile b/modules/tests/Makefile index 5c94e549..b0bfb031 100644 --- a/modules/tests/Makefile +++ b/modules/tests/Makefile @@ -62,6 +62,13 @@ $(out)/%.so: $(out)/%.o $(out)/tests/tst-non-fpic.o: CXXFLAGS:=$(subst -fPIC,-mcmodel=large,$(CXXFLAGS)) +$(out)/tests/tst-getopt-pie.o: CXXFLAGS:=$(subst -fPIC,-fpie,$(CXXFLAGS)) +$(out)/tests/tst-getopt-pie.o: $(src)/tests/tst-getopt.cc + $(makedir) + $(call quiet, $(CXX) $(CXXFLAGS) -c -o $@ $<, CXX $*.cc) +$(out)/tests/tst-getopt-pie.so: $(out)/tests/tst-getopt-pie.o + $(call quiet, $(CXX) $(CXXFLAGS) -pie -o $@ $< $(LIBS), LD $*.so) + # The rofs test image mounts /tmp as ramfs and 4 tests that exercise file system # fail due to some unresolved bugs or other shortcomings of the ramfs implementation # and are temporarily removed from the rofs-only-tests list. The tests tst-readdir.so @@ -117,7 +124,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so tst-bsd-evh.so \ tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so \ tst-sigaltstack.so tst-fread.so tst-tcp-cork.so tst-tcp-v6.so \ tst-calloc.so tst-crypt.so tst-non-fpic.so tst-small-malloc.so \ - tst-mmx-fpu.so + tst-mmx-fpu.so tst-getopt.so tst-getopt-pie.so # libstatic-thread-variable.so tst-static-thread-variable.so \ tests += testrunner.so diff --git a/tests/tst-getopt.cc b/tests/tst-getopt.cc new file mode 100644 index 00000000..7e70f754 --- /dev/null +++ b/tests/tst-getopt.cc @@ -0,0 +1,232 @@ +/* + * Copyright (C) 2019 Waldek Kozaczuk + * + * This work is open source software, licensed under the terms of the + * BSD license as described in the LICENSE file in the top-level directory. + * + * This source code is loosely based on the examples from GNU libs manuals + * https://www.gnu.org/software/libc/manual/html_node/Getopt-Long-Option-Example.html + * and https://www.gnu.org/software/libc/manual/html_node/Example-of-Getopt.html. + */ + +#include <getopt.h> +#include <ctype.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <cassert> +#include <cstring> + +void +test_getopt(int argc, char *const argv[], int expected_aflag, int expected_bflag, + const char *expected_cvalue, const char *expected_non_option_arg) { + + int aflag = 0; + int bflag = 0; + char *cvalue = NULL; + int c; + + printf("Running test: %s ...\n", argv[0]); + + opterr = 0; + optind = 0; + + while ((c = getopt(argc, argv, "abc:")) != -1) + switch (c) { + case 'a': + aflag = 1; + break; + case 'b': + bflag = 1; + break; + case 'c': + cvalue = optarg; + break; + case '?': + if (optopt == 'c') + fprintf(stderr, "Option -%c requires an argument.\n", optopt); + else if (isprint (optopt)) + fprintf(stderr, "Unknown option `-%c'.\n", optopt); + else + fprintf(stderr, + "Unknown option character `\\x%x'.\n", + optopt); + assert(0); + default: + assert(0); + } + + assert(expected_aflag == aflag); + assert(expected_bflag == bflag); + if (expected_cvalue && cvalue) { + assert(strcmp(expected_cvalue, cvalue) == 0); + } else { + assert(!cvalue && !expected_cvalue); + } + + if (expected_non_option_arg) { + assert(optind + 1 == argc); + assert(strcmp(expected_non_option_arg, argv[optind]) == 0); + } else { + assert(optind == argc); + } +} + +static int verbose_flag; + +void +test_getopt_long(int argc, char *const argv[], int expected_aflag, int expected_bflag, const char *expected_cvalue, + const char *expected_dvalue, const char *expected_fvalue, int expected_verbose) { + int c; + int aflag = 0; + int bflag = 0; + char *cvalue = NULL; + char *dvalue = NULL; + char *fvalue = NULL; + + printf("Running test: %s ...\n", argv[0]); + + opterr = 0; + optind = 0; + + while (1) { + struct option long_options[] = + { + /* These options set a flag. */ + {"verbose", no_argument, &verbose_flag, 1}, + {"brief", no_argument, &verbose_flag, 0}, + /* These options don’t set a flag. + We distinguish them by their indices. */ + {"add", no_argument, 0, 'a'}, + {"append", no_argument, 0, 'b'}, + {"delete", required_argument, 0, 'd'}, + {"create", required_argument, 0, 'c'}, + {"file", required_argument, 0, 'f'}, + {0, 0, 0, 0} + }; + /* getopt_long stores the option index here. */ + int option_index = 0; + + c = getopt_long(argc, argv, "abc:d:f:", + long_options, &option_index); + + /* Detect the end of the options. */ + if (c == -1) + break; + + switch (c) { + case 0: + /* If this option set a flag, do nothing else now. */ + if (long_options[option_index].flag != 0) + break; + printf("option %s", long_options[option_index].name); + if (optarg) + printf(" with arg %s", optarg); + printf("\n"); + break; + + case 'a': + aflag = 1; + break; + + case 'b': + bflag = 1; + break; + + case 'c': + printf("option -c with value `%s'\n", optarg); + cvalue = optarg; + break; + + case 'd': + printf("option -d with value `%s'\n", optarg); + dvalue = optarg; + break; + + case 'f': + printf("option -f with value `%s'\n", optarg); + fvalue = optarg; + break; + + case '?': + /* getopt_long already printed an error message. */ + assert(0); + + default: + assert(0); + } + } + + assert(expected_aflag == aflag); + assert(expected_bflag == bflag); + + if (expected_cvalue && cvalue) { + assert(strcmp(expected_cvalue, cvalue) == 0); + } else { + assert(!cvalue && !expected_cvalue); + } + + if (expected_dvalue && dvalue) { + assert(strcmp(expected_dvalue, dvalue) == 0); + } else { + assert(!dvalue && !expected_dvalue); + } + + if (expected_fvalue && fvalue) { + assert(strcmp(expected_fvalue, fvalue) == 0); + } else { + assert(!fvalue && !expected_fvalue); + } + + assert(verbose_flag == expected_verbose); + + if (optind < argc) { + printf("non-option ARGV-elements: "); + while (optind < argc) + printf("%s ", argv[optind++]); + putchar('\n'); + } +} + +int +main(int argc, char *argv[]) { + char *const test1[] = {(char *) "tst-getopt1", nullptr}; + test_getopt(1, test1, 0, 0, nullptr, nullptr); + + char *const test2[] = {(char *) "tst-getopt2", (char *) "-a", (char *) "-b", nullptr}; + test_getopt(3, test2, 1, 1, nullptr, nullptr); + + char *const test3[] = {(char *) "tst-getopt3", (char *) "-ab", nullptr}; + test_getopt(2, test3, 1, 1, nullptr, nullptr); + + char *const test4[] = {(char *) "tst-getopt4", (char *) "-c", (char *) "foo", nullptr}; + test_getopt(3, test4, 0, 0, "foo", nullptr); + + char *const test5[] = {(char *) "tst-getopt5", (char *) "-cfoo", nullptr}; + test_getopt(2, test5, 0, 0, "foo", nullptr); + + char *const test6[] = {(char *) "tst-getopt6", (char *) "arg1", nullptr}; + test_getopt(2, test6, 0, 0, nullptr, "arg1"); + + char *const test7[] = {(char *) "tst-getopt7", (char *) "-a", (char *) "arg1", nullptr}; + test_getopt(3, test7, 1, 0, nullptr, "arg1"); + + char *const test8[] = {(char *) "tst-getopt8", (char *) "-c", (char *) "foo", (char *) "arg1", nullptr}; + test_getopt(4, test8, 0, 0, "foo", "arg1"); + + char *const test9[] = {(char *) "tst-getopt9", (char *) "-a", (char *) "--", (char *) "-b", nullptr}; + test_getopt(4, test9, 1, 0, nullptr, "-b"); + + char *const test10[] = {(char *) "tst-getopt10", (char *) "-a", (char *) "-", nullptr}; + test_getopt(3, test10, 1, 0, nullptr, "-"); + + char *const long_test1[] = {(char *) "tst-getopt_long1", (char *) "-a", (char *) "--create", (char *) "bula", + nullptr}; + test_getopt_long(4, long_test1, 1, 0, "bula", nullptr, nullptr, 0); + + char *const long_test2[] = {(char *) "tst-getopt_long2", (char *) "-a", (char *) "--create", (char *) "bula", + (char *) "--append", (char *) "-f", (char *) "x.txt", (char *) "--verbose", nullptr}; + test_getopt_long(8, long_test2, 1, 1, "bula", nullptr, "x.txt", 1); + + return 0; +} -- 2.20.1 -- You received this message because you are subscribed to the Google Groups "OSv Development" 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.
