Michael Vogt
777d9c10ad
The fix in 8f1751a111 made me wonder if we could automatically detect
when pointers are accessed but when this might not be safe. Systemd
is already using a lot of `assert(dst)` and this change now forces
us to use them.
So this commit (ab)uses coccinelle to flag any pointer parameter
dereference not preceded by assert(param), ASSERT_PTR(param), or an
explicit NULL check. It adds integration into meson as a new "coccinelle"
test suite (just like clang-tidy) and is run in CI. The check is not
perfect but seems a reasonable heuristic.
For this RFC commit it is scoped to a subset, it excludes 25 dirs right
now and includes around 100. About 300 warnings left. Busywork that I am
happy to do if there is agreement that it is worth it.
With this in place we would have caught the bug from 8f1751a111 in CI:
```
FAIL: check-pointer-deref.cocci found issues in systemd/src/boot:
diff -u -p systemd/src/boot/measure.c /tmp/nothing/measure.c
--- systemd/src/boot/measure.c
+++ /tmp/nothing/measure.c
@@ -312,7 +312,6 @@ EFI_STATUS tpm_log_tagged_event(
if (err != EFI_SUCCESS)
return err;
- *ret_measured = true;
return EFI_SUCCESS;
}
```
This also adds a new POINTER_MAY_BE_NULL() for the cases when the
called function will do the NULL check (like `iovec_is_set()`).
29 lines
841 B
Bash
Executable File
29 lines
841 B
Bash
Executable File
#!/usr/bin/env bash
|
|
# SPDX-License-Identifier: LGPL-2.1-or-later
|
|
set -eu
|
|
set -o pipefail
|
|
|
|
SRC_DIR="${1:?}"
|
|
COCCI_DIR="${2:?}"
|
|
|
|
FOUND=0
|
|
|
|
for cocci in "$COCCI_DIR"/check-*.cocci; do
|
|
[[ -f "$cocci" ]] || continue
|
|
output=$(spatch --very-quiet --sp-file "$cocci" --dir "$SRC_DIR" 2>&1)
|
|
if [[ -n "$output" ]]; then
|
|
echo "FAIL: $(basename "$cocci") found issues in $SRC_DIR:"
|
|
echo "$output"
|
|
FOUND=1
|
|
fi
|
|
done
|
|
|
|
if [[ "$FOUND" -ne 0 ]]; then
|
|
echo ""
|
|
echo "Coccinelle check(s) failed. For each flagged dereference, either:"
|
|
echo " - Add assert(param)/ASSERT_PTR(param) at the top of the function (if the parameter must not be NULL)"
|
|
echo " - Add an if (param) guard before the dereference (if NULL is valid)"
|
|
echo " - Add POINTER_MAY_BE_NULL(param) if NULL is okay for param"
|
|
exit 1
|
|
fi
|