1
0
mirror of https://github.com/systemd/systemd synced 2026-04-09 16:44:51 +02:00

Compare commits

...

9 Commits

Author SHA1 Message Date
Frantisek Sumsal
ab9e3bfef6 ci: consider cryptolib in the group identifier
otherwise we end up with more than one job with the same identifier in
one run, causing some of them to get cancelled unexpectedly.

A quick follow-up to 85bd394df57fe45c2873605e2c1d1d79e83e853d.
2021-12-03 20:25:06 +00:00
Luca Boccassi
09dfd918ef
Merge pull request #21607 from mrc0mmand/ci-install-libbpf
ci: run build test with BPF-related stuff as well
2021-12-03 18:37:33 +00:00
Luca Boccassi
86167587c5
Merge pull request #21582 from mrc0mmand/lgtm-uninitialized
lgtm: enable more queries
2021-12-03 18:25:19 +00:00
Frantisek Sumsal
9371d44afe ci: install libbpf 2021-12-03 16:30:56 +01:00
Frantisek Sumsal
466e63a453 analyze: fix build with -Db_ndebug=true 2021-12-03 16:22:52 +01:00
Frantisek Sumsal
6108ab163e meson: support versioned llvm binaries in BPF detection 2021-12-03 16:22:52 +01:00
Frantisek Sumsal
38f36b9f34 lgtm: enable more (and potentially useful) queries
Not all available queries on LGTM are enabled by default, but some of
the excluded ones might come in handy, hence let's enable them
explicitly.
2021-12-02 17:22:49 +01:00
Frantisek Sumsal
c7d70210fa lgtm: don't treat the custom note as a list of tags
Just a cosmetic change.
2021-12-02 16:56:54 +01:00
Frantisek Sumsal
863bff7548 lgtm: detect uninitialized variables using the __cleanup__ attribute
This is a slightly modified version of the original
`cpp/uninitialized-local` CodeQL query which focuses only on variables
using the cleanup macros. Since this has proven to cause issues in the
past, let's panic on every uninitialized variable using any of the
cleanup macros (as long as they're written using the __cleanup__
attribute).

Some test results from a test I used when writing the query:

```
 #define _cleanup_foo_ __attribute__((__cleanup__(foo)))
 #define _cleanup_(x) __attribute__((__cleanup__(x)))

 static inline void freep(void *p) {
         *(void**)p = mfree(*(void**) p);
 }

 #define _cleanup_free_ _cleanup_(freep)

 static inline void foo(char **p) {
     if (*p)
         *p = free(*p);
 }

 int main(void) {
     __attribute__((__cleanup__(foo))) char *a;
     char *b;
     _cleanup_foo_ char *c;
     char **d;
     _cleanup_free_ char *e;
     int r;

     r = fun(&e);
     if (r < 0)
         return 1;

     puts(a);
     puts(b);
     puts(c);
     puts(*d);
     puts(e);

     return 0;
 }
```

```
+| test.c:23:14:23:14 | e | The variable $@ may not be initialized here, but has a cleanup handler. | test.c:20:26:20:26 | e | e |
+| test.c:27:10:27:10 | a | The variable $@ may not be initialized here, but has a cleanup handler. | test.c:16:45:16:45 | a | a |
+| test.c:29:10:29:10 | c | The variable $@ may not be initialized here, but has a cleanup handler. | test.c:18:25:18:25 | c | c |
```
2021-12-02 16:56:54 +01:00
7 changed files with 145 additions and 13 deletions

View File

@ -27,6 +27,7 @@ PACKAGES=(
itstool
kbd
libblkid-dev
libbpf-dev
libcap-dev
libcurl4-gnutls-dev
libfdisk-dev
@ -48,8 +49,8 @@ PACKAGES=(
net-tools
perl
python3-evdev
python3-lxml
python3-jinja2
python3-lxml
python3-pip
python3-pyparsing
python3-setuptools

View File

@ -16,7 +16,7 @@ jobs:
build:
runs-on: ubuntu-20.04
concurrency:
group: ${{ github.workflow }}-${{ matrix.run_phase }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ matrix.run_phase }}-${{ matrix.cryptolib }}-${{ github.ref }}
cancel-in-progress: true
strategy:
fail-fast: false

View File

@ -1,6 +1,27 @@
---
# vi: ts=2 sw=2 et:
# Explicitly enable certain checks which are hidden by default
queries:
- include: cpp/bad-strncpy-size
- include: cpp/declaration-hides-variable
- include: cpp/inconsistent-null-check
- include: cpp/mistyped-function-arguments
- include: cpp/nested-loops-with-same-variable
- include: cpp/sizeof-side-effect
- include: cpp/suspicious-pointer-scaling
- include: cpp/suspicious-pointer-scaling-void
- include: cpp/suspicious-sizeof
- include: cpp/unsafe-strcat
- include: cpp/unsafe-strncat
- include: cpp/unsigned-difference-expression-compared-zero
- include: cpp/unused-local-variable
- include:
tags:
- "security"
- "correctness"
severity: "error"
extraction:
cpp:
prepare:

View File

@ -1,15 +1,17 @@
/**
* @name Use of potentially dangerous function
* @description Certain standard library functions are dangerous to call.
* @kind problem
* @problem.severity error
* @precision high
* @id cpp/potentially-dangerous-function
* @tags reliability
* security
* vi: sw=2 ts=2 et syntax=ql:
*
* Borrowed from
* https://github.com/Semmle/ql/blob/master/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql
*
* @name Use of potentially dangerous function
* @description Certain standard library functions are dangerous to call.
* @id cpp/potentially-dangerous-function
* @kind problem
* @problem.severity error
* @precision high
* @tags reliability
* security
*/
import cpp

View File

@ -0,0 +1,99 @@
/**
* vi: sw=2 ts=2 et syntax=ql:
*
* Based on cpp/uninitialized-local.
*
* @name Potentially uninitialized local variable using the cleanup attribute
* @description Running the cleanup handler on a possibly uninitialized variable
* is generally a bad idea.
* @id cpp/uninitialized-local-with-cleanup
* @kind problem
* @problem.severity error
* @precision high
* @tags security
*/
import cpp
import semmle.code.cpp.controlflow.StackVariableReachability
/**
* Auxiliary predicate: Types that don't require initialization
* before they are used, since they're stack-allocated.
*/
predicate allocatedType(Type t) {
/* Arrays: "int foo[1]; foo[0] = 42;" is ok. */
t instanceof ArrayType
or
/* Structs: "struct foo bar; bar.baz = 42" is ok. */
t instanceof Class
or
/* Typedefs to other allocated types are fine. */
allocatedType(t.(TypedefType).getUnderlyingType())
or
/* Type specifiers don't affect whether or not a type is allocated. */
allocatedType(t.getUnspecifiedType())
}
/**
* A declaration of a local variable using __attribute__((__cleanup__(x)))
* that leaves the variable uninitialized.
*/
DeclStmt declWithNoInit(LocalVariable v) {
result.getADeclaration() = v and
not exists(v.getInitializer()) and
/* The variable has __attribute__((__cleanup__(...))) set */
v.getAnAttribute().hasName("cleanup") and
/* The type of the variable is not stack-allocated. */
exists(Type t | t = v.getType() | not allocatedType(t))
}
class UninitialisedLocalReachability extends StackVariableReachability {
UninitialisedLocalReachability() { this = "UninitialisedLocal" }
override predicate isSource(ControlFlowNode node, StackVariable v) { node = declWithNoInit(v) }
/* Note: _don't_ use the `useOfVarActual()` predicate here (and a couple of lines
* below), as it assumes that the callee always modifies the variable if
* it's passed to the function.
*
* i.e.:
* _cleanup_free char *x;
* fun(&x);
* puts(x);
*
* `useOfVarActual()` won't treat this an an uninitialized read even if the callee
* doesn't modify the argument, however, `useOfVar()` will
*/
override predicate isSink(ControlFlowNode node, StackVariable v) { useOfVar(v, node) }
override predicate isBarrier(ControlFlowNode node, StackVariable v) {
// only report the _first_ possibly uninitialized use
useOfVar(v, node) or
definitionBarrier(v, node)
}
}
pragma[noinline]
predicate containsInlineAssembly(Function f) { exists(AsmStmt s | s.getEnclosingFunction() = f) }
/**
* Auxiliary predicate: List common exceptions or false positives
* for this check to exclude them.
*/
VariableAccess commonException() {
// If the uninitialized use we've found is in a macro expansion, it's
// typically something like va_start(), and we don't want to complain.
result.getParent().isInMacroExpansion()
or
result.getParent() instanceof BuiltInOperation
or
// Finally, exclude functions that contain assembly blocks. It's
// anyone's guess what happens in those.
containsInlineAssembly(result.getEnclosingFunction())
}
from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va
where
r.reaches(_, v, va) and
not va = commonException()
select va, "The variable $@ may not be initialized here, but has a cleanup handler.", v, v.getName()

View File

@ -992,8 +992,17 @@ conf.set10('HAVE_LIBBPF', libbpf.found())
if want_bpf_framework == 'false'
conf.set10('BPF_FRAMEWORK', 0)
else
clang = find_program('clang', required : bpf_framework_required)
llvm_strip = find_program('llvm-strip', required : bpf_framework_required)
# Support 'versioned' clang/llvm-strip binaries, as seen on Debian/Ubuntu
# (like clang-10/llvm-strip-10)
clang_bin = cc.get_id() == 'clang' ? cc.cmd_array()[0] : 'clang'
clang = find_program(clang_bin, required : bpf_framework_required)
if clang.found()
llvm_strip_bin = run_command(clang, '--print-prog-name', 'llvm-strip',
check : true).stdout().strip()
else
llvm_strip_bin = 'llvm-strip'
endif
llvm_strip = find_program(llvm_strip_bin, required : bpf_framework_required)
# Debian installs this in /usr/sbin/ which is not in $PATH.
# We check for 'bpftool' first, honouring $PATH, and in /usr/sbin/ for Debian.

View File

@ -1959,7 +1959,7 @@ static int dump_filesystems(int argc, char *argv[], void *userdata) {
const statfs_f_type_t *magic;
bool is_primary = false;
assert(fs_type_from_string(*filesystem, &magic) >= 0);
assert_se(fs_type_from_string(*filesystem, &magic) >= 0);
for (size_t i = 0; magic[i] != 0; i++) {
const char *primary;