Fixes and cleanups

Author: fornwall

.clang-tidy

@@ -1,2 +1,2 @@
-Checks: '-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-valist.Uninitialized'
+Checks: '-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-valist.Uninitialized,-clang-diagnostic-tautological-pointer-compare'
 

.github/workflows/ci.yml

@@ -13,9 +13,9 @@ jobs:
       - uses: actions/checkout@v4
       - uses: Homebrew/actions/setup-homebrew@master
       - run: brew install clang-format
-      - run: make
-      - run: make check
-      - run: make unit-test
+      - run: make CC=clang
+      - run: make check CC=clang
+      - run: make unit-test CC=clang
 
   actionlint:
     runs-on: ubuntu-latest

Makefile

@@ -1,25 +1,39 @@
+CC ?= clang
 TERMUX_BASE_DIR ?= /data/data/com.termux/files
-CFLAGS += -Wall -Wextra -Werror -Wshadow -O2 -fvisibility=hidden
+CFLAGS += -Wall -Wextra -Werror -Wshadow -fvisibility=hidden -std=c17 -Wno-error=tautological-pointer-compare
 C_SOURCE := src/termux-exec.c src/exec-variants.c
 CLANG_FORMAT := clang-format --sort-includes --style="{ColumnLimit: 120}" $(C_SOURCE)
 CLANG_TIDY ?= clang-tidy
 
+ifeq ($(SANITIZE),1)
+  CFLAGS += -O1 -g -fsanitize=address -fno-omit-frame-pointer
+else
+  CFLAGS += -O2
+endif
+
 libtermux-exec.so: $(C_SOURCE)
 	$(CC) $(CFLAGS) $(LDFLAGS) $(C_SOURCE) -DTERMUX_PREFIX=\"$(TERMUX_PREFIX)\" -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" -shared -fPIC -o libtermux-exec.so
 
 tests/fexecve: tests/fexecve.c
 	$(CC) $(CFLAGS) -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" $< -o $@
 
+$(TERMUX_BASE_DIR)/usr/bin/termux-exec-test-print-argv0: tests/print-argv0.c
+	$(CC) $(CFLAGS) $< -o $@
+
 clean:
-	rm -f libtermux-exec.so tests/*-actual test-binary
+	rm -f libtermux-exec.so tests/*-actual test-binary $(TERMUX_BASE_DIR)/usr/bin/termux-exec-test-print-argv0
 
 install: libtermux-exec.so
 	install libtermux-exec.so $(DESTDIR)$(PREFIX)/lib/libtermux-exec.so
 
 uninstall:
 	rm -f $(DESTDIR)$(PREFIX)/lib/libtermux-exec.so
 
-on-device-tests: libtermux-exec.so tests/fexecve
+on-device-tests:
+	make clean
+	ASAN_OPTIONS=symbolize=0,detect_leaks=0 make SANITIZE=1 on-device-tests-internal
+
+on-device-tests-internal: libtermux-exec.so tests/fexecve $(TERMUX_BASE_DIR)/usr/bin/termux-exec-test-print-argv0
 	@LD_PRELOAD=${CURDIR}/libtermux-exec.so ./run-tests.sh
 
 format:
@@ -36,6 +50,6 @@ deb: libtermux-exec.so
 	termux-create-package termux-exec-debug.json
 
 unit-test: test-binary
-	./test-binary
+	ASAN_OPTIONS=symbolize=0 ./test-binary
 
-.PHONY: deb clean install uninstall test format check-format test
+.PHONY: clean install uninstall on-device-tests on-device-tests-internal format check deb unit-test

README.md

@@ -1,5 +1,5 @@
 # termux-exec
-A `execve()` wrapper to fix two problems with exec-ing files in Termux.
+A `execve(3)` (and exec family of functions) wrapper to fix two problems with exec-ing files in Termux.
 
 # Problem 1: Cannot execute files not part of the APK
 Android 10 started blocking executing files under the app data directory, as
@@ -14,7 +14,7 @@ While there is merit in that general principle, this prevents using Termux and A
 as a general computing device, where it should be possible for users to create executable
 scripts and binaries.
 
-# Solution 1: Cannot execute files not part of the APK
+## Solution
 Create an `exec` interceptor using [LD_PRELOAD](https://en.wikipedia.org/wiki/DLL_injection#Approaches_on_Unix-like_systems),
 that instead of executing an ELF file directly, executes `/system/bin/linker64 /path/to/elf`.
 Explanation follows below.
@@ -62,11 +62,6 @@ We could also consider patching this exec interception into the build process of
 **NOTE**: The above example used `/system/bin/linker64` - on 32-bit systems, the corresponding
 path is `/system/bin/linker`.
 
-**NOTE**: While this circumvents the technical restriction, it still might be considered
-violating [Google Play policy](https://support.google.com/googleplay/android-developer/answer/9888379).
-So this workaround is not guaranteed to enable Play store distribution of Termux - but it's
-worth an attempt, and regardless of Play store distribution, updating the targetSdk is necessary.
-
 # Problem 2: Shebang paths
 A lot of Linux software is written with the assumption that `/bin/sh`, `/usr/bin/env`
 and similar file exists. This is not the case on Android where neither `/bin/` nor `/usr/`
@@ -75,19 +70,16 @@ exists.
 When building packages for Termux those hard-coded assumptions are patched away - but this
 does not help with installing scripts and programs from other sources than Termux packages.
 
-# Solution 2: Shebang paths
+## Solution
 Create an `execve()` wrapper that rewrites calls to execute files under `/bin/` and `/usr/bin`
 into the matching Termux executables under `$PREFIX/bin/` and inject that into processes
 using `LD_PRELOAD`.
 
-# How to install
-1. Install with `pkg install termux-exec`.
-2. Exit your current session and start a new one.
-3. From now on shebangs such as `/bin/sh` and `/usr/bin/env python` should work.
-
-# Where is LD_PRELOAD set?
-The `$PREFIX/bin/login` program which is used to create new Termux sessions checks for
-`$PREFIX/lib/libtermux-exec.so` and if so sets up `LD_PRELOAD` before launching the login shell.
+# Usage in Termux
+This `termux-exec` package comes preinstalled as an essential package in Termux. Each time a
+process is spawned by the app, the `LD_PRELOAD=$PREFIX/lib/libtermux-exec.so` environment variable
+is setup by the Android app. This environment variable needs to be kept at all times when executing
+files inside Termux.
 
-Soon, when making a switch to target Android 10+, this will be setup by the Termux app even before
-launching any process, as `LD_PRELOAD` will be necessary for anything non-system to execute.
+In the future the linker `--wrap` functionaliy might be used to avoid having to rely on `LD_PRELOAD`
+always being present.

src/exec-variants.c

@@ -39,7 +39,10 @@ __attribute__((visibility("default"))) int execvp(const char *name, char *const
 }
 
 __attribute__((visibility("default"))) int execvpe(const char *name, char *const *argv, char *const *envp) {
-  // if (name == NULL || *name == '\0') { errno = ENOENT; return -1; }
+  if (name == NULL || *name == '\0') {
+    errno = ENOENT;
+    return -1;
+  }
 
   // If it's an absolute or relative path name, it's easy.
   if (strchr(name, '/') && execve(name, argv, envp) == -1) {