From 3d3dc89fb25868e8038ecac8d5aef0603bdfaaa2 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: Jan 19 2022 21:25:15 +0000 Subject: import systemd-239-55.el8 --- diff --git a/SOURCES/0680-lgtm-detect-uninitialized-variables-using-the-__clea.patch b/SOURCES/0680-lgtm-detect-uninitialized-variables-using-the-__clea.patch new file mode 100644 index 0000000..d8f878e --- /dev/null +++ b/SOURCES/0680-lgtm-detect-uninitialized-variables-using-the-__clea.patch @@ -0,0 +1,171 @@ +From cecb3cc06f6025835324c1837c03def1d9be8eb1 Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Wed, 1 Dec 2021 21:31:43 +0100 +Subject: [PATCH] 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 | +``` + +(cherry picked from commit 863bff75488d33f519deea6390988f3d9d72e6de) + +Related: #2017033 +--- + .../UninitializedVariableWithCleanup.ql | 99 +++++++++++++++++++ + 1 file changed, 99 insertions(+) + create mode 100644 .lgtm/cpp-queries/UninitializedVariableWithCleanup.ql + +diff --git a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql +new file mode 100644 +index 0000000000..6bf0ae01eb +--- /dev/null ++++ b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql +@@ -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() diff --git a/SOURCES/0681-lgtm-replace-the-query-used-for-looking-for-fgets-wi.patch b/SOURCES/0681-lgtm-replace-the-query-used-for-looking-for-fgets-wi.patch new file mode 100644 index 0000000..acccb51 --- /dev/null +++ b/SOURCES/0681-lgtm-replace-the-query-used-for-looking-for-fgets-wi.patch @@ -0,0 +1,84 @@ +From c4a34b71d4f51f071f7a722059e36388b41d30e4 Mon Sep 17 00:00:00 2001 +From: Evgeny Vereshchagin +Date: Mon, 11 Mar 2019 21:05:13 +0100 +Subject: [PATCH] lgtm: replace the query used for looking for fgets with a + more general query + +to make it easier to comlain about `strtok` :-) + +Inspired by https://github.com/systemd/systemd/pull/11963, which, in turn, +was prompted by https://github.com/systemd/systemd/pull/11555. + +(cherry picked from commit 7ba5ded9dbd7737bc368521f5ea7c90e5b06ab3e) + +Related: #2017033 +--- + .../PotentiallyDangerousFunction.ql | 30 +++++++++++++++++++ + .lgtm/cpp-queries/fgets.ql | 21 ------------- + 2 files changed, 30 insertions(+), 21 deletions(-) + create mode 100644 .lgtm/cpp-queries/PotentiallyDangerousFunction.ql + delete mode 100644 .lgtm/cpp-queries/fgets.ql + +diff --git a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +new file mode 100644 +index 0000000000..ba80f4ad8c +--- /dev/null ++++ b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +@@ -0,0 +1,30 @@ ++/** ++ * @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 ++ * ++ * Borrowed from ++ * https://github.com/Semmle/ql/blob/master/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql ++ */ ++import cpp ++ ++predicate potentiallyDangerousFunction(Function f, string message) { ++ ( ++ f.getQualifiedName() = "fgets" and ++ message = "Call to fgets is potentially dangerous. Use read_line() instead." ++ ) or ( ++ f.getQualifiedName() = "strtok" and ++ message = "Call to strtok is potentially dangerous. Use extract_first_word() instead." ++ ) ++} ++ ++from FunctionCall call, Function target, string message ++where ++ call.getTarget() = target and ++ potentiallyDangerousFunction(target, message) ++select call, message +diff --git a/.lgtm/cpp-queries/fgets.ql b/.lgtm/cpp-queries/fgets.ql +deleted file mode 100644 +index a4181e4f3d..0000000000 +--- a/.lgtm/cpp-queries/fgets.ql ++++ /dev/null +@@ -1,21 +0,0 @@ +-/** +- * @name Use of fgets() +- * @description fgets() is dangerous to call. Use read_line() instead. +- * @kind problem +- * @problem.severity error +- * @precision high +- * @id cpp/fgets +- * @tags reliability +- * security +- */ +-import cpp +- +-predicate dangerousFunction(Function function) { +- exists (string name | name = function.getQualifiedName() | +- name = "fgets") +-} +- +-from FunctionCall call, Function target +-where call.getTarget() = target +- and dangerousFunction(target) +-select call, target.getQualifiedName() + " is potentially dangerous" diff --git a/SOURCES/0682-lgtm-beef-up-list-of-dangerous-questionnable-API-cal.patch b/SOURCES/0682-lgtm-beef-up-list-of-dangerous-questionnable-API-cal.patch new file mode 100644 index 0000000..03e3939 --- /dev/null +++ b/SOURCES/0682-lgtm-beef-up-list-of-dangerous-questionnable-API-cal.patch @@ -0,0 +1,48 @@ +From 8b60932555141e1fe61a343863eae7655c2449a9 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Tue, 2 Apr 2019 12:43:47 +0200 +Subject: [PATCH] lgtm: beef up list of dangerous/questionnable API calls not + to make + +(cherry picked from commit 9b4805421eb2a7319f6507a26febfb9d2cdc3a93) + +Related: #2017033 +--- + .../PotentiallyDangerousFunction.ql | 22 +++++++++++++++++-- + 1 file changed, 20 insertions(+), 2 deletions(-) + +diff --git a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +index ba80f4ad8c..cd0284b37a 100644 +--- a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql ++++ b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +@@ -16,10 +16,28 @@ import cpp + predicate potentiallyDangerousFunction(Function f, string message) { + ( + f.getQualifiedName() = "fgets" and +- message = "Call to fgets is potentially dangerous. Use read_line() instead." ++ message = "Call to fgets() is potentially dangerous. Use read_line() instead." + ) or ( + f.getQualifiedName() = "strtok" and +- message = "Call to strtok is potentially dangerous. Use extract_first_word() instead." ++ message = "Call to strtok() is potentially dangerous. Use extract_first_word() instead." ++ ) or ( ++ f.getQualifiedName() = "strsep" and ++ message = "Call to strsep() is potentially dangerous. Use extract_first_word() instead." ++ ) or ( ++ f.getQualifiedName() = "dup" and ++ message = "Call to dup() is potentially dangerous. Use fcntl(fd, FD_DUPFD_CLOEXEC, 3) instead." ++ ) or ( ++ f.getQualifiedName() = "htonl" and ++ message = "Call to htonl() is confusing. Use htobe32() instead." ++ ) or ( ++ f.getQualifiedName() = "htons" and ++ message = "Call to htons() is confusing. Use htobe16() instead." ++ ) or ( ++ f.getQualifiedName() = "ntohl" and ++ message = "Call to ntohl() is confusing. Use be32toh() instead." ++ ) or ( ++ f.getQualifiedName() = "ntohs" and ++ message = "Call to ntohs() is confusing. Use be16toh() instead." + ) + } + diff --git a/SOURCES/0683-lgtm-warn-about-strerror-use.patch b/SOURCES/0683-lgtm-warn-about-strerror-use.patch new file mode 100644 index 0000000..2661bfe --- /dev/null +++ b/SOURCES/0683-lgtm-warn-about-strerror-use.patch @@ -0,0 +1,26 @@ +From af6eac25456d4ca7e8233e00aec7531e640f17af Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Fri, 5 Apr 2019 15:31:34 +0200 +Subject: [PATCH] lgtm: warn about strerror() use + +(cherry picked from commit 9ff46eded2b99d244455467eb55c0ff3f51c5362) + +Related: #2017033 +--- + .lgtm/cpp-queries/PotentiallyDangerousFunction.ql | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +index cd0284b37a..96712cf1c6 100644 +--- a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql ++++ b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +@@ -38,6 +38,9 @@ predicate potentiallyDangerousFunction(Function f, string message) { + ) or ( + f.getQualifiedName() = "ntohs" and + message = "Call to ntohs() is confusing. Use be16toh() instead." ++ ) or ( ++ f.getQualifiedName() = "strerror" and ++ message = "Call to strerror() is not thread-safe. Use strerror_r() or printf()'s %m format string instead." + ) + } + diff --git a/SOURCES/0684-lgtm-complain-about-accept-people-should-use-accept4.patch b/SOURCES/0684-lgtm-complain-about-accept-people-should-use-accept4.patch new file mode 100644 index 0000000..7c3b69c --- /dev/null +++ b/SOURCES/0684-lgtm-complain-about-accept-people-should-use-accept4.patch @@ -0,0 +1,27 @@ +From bfa090ce83f2b0734c526a4426a20f6f0f943aa0 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Wed, 10 Apr 2019 19:36:40 +0200 +Subject: [PATCH] lgtm: complain about accept() [people should use accept4() + instead, due to O_CLOEXEC] + +(cherry picked from commit e2d0fa6feb3797246c8bfda3db45a2f5b62e1b5b) + +Related: #2017033 +--- + .lgtm/cpp-queries/PotentiallyDangerousFunction.ql | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +index 96712cf1c6..865330430d 100644 +--- a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql ++++ b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +@@ -41,6 +41,9 @@ predicate potentiallyDangerousFunction(Function f, string message) { + ) or ( + f.getQualifiedName() = "strerror" and + message = "Call to strerror() is not thread-safe. Use strerror_r() or printf()'s %m format string instead." ++ ) or ( ++ f.getQualifiedName() = "accept" and ++ message = "Call to accept() is not O_CLOEXEC-safe. Use accept4() instead." + ) + } + diff --git a/SOURCES/0685-lgtm-don-t-treat-the-custom-note-as-a-list-of-tags.patch b/SOURCES/0685-lgtm-don-t-treat-the-custom-note-as-a-list-of-tags.patch new file mode 100644 index 0000000..0eb213f --- /dev/null +++ b/SOURCES/0685-lgtm-don-t-treat-the-custom-note-as-a-list-of-tags.patch @@ -0,0 +1,40 @@ +From 6eeaef95566e6d85e714280c412e5df347838e34 Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Thu, 2 Dec 2021 16:55:17 +0100 +Subject: [PATCH] lgtm: don't treat the custom note as a list of tags + +Just a cosmetic change. + +(cherry picked from commit c7d70210fa45c3210b8b1eda51bc0f6d68bd8392) + +Related: #2017033 +--- + .lgtm/cpp-queries/PotentiallyDangerousFunction.ql | 10 ++++++---- + 1 file changed, 6 insertions(+), 4 deletions(-) + +diff --git a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +index 865330430d..39e8dddd13 100644 +--- a/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql ++++ b/.lgtm/cpp-queries/PotentiallyDangerousFunction.ql +@@ -1,15 +1,17 @@ + /** ++ * 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 +- * @id cpp/potentially-dangerous-function + * @tags reliability + * security +- * +- * Borrowed from +- * https://github.com/Semmle/ql/blob/master/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql + */ + import cpp + diff --git a/SOURCES/0686-lgtm-ignore-certain-cleanup-functions.patch b/SOURCES/0686-lgtm-ignore-certain-cleanup-functions.patch new file mode 100644 index 0000000..0793564 --- /dev/null +++ b/SOURCES/0686-lgtm-ignore-certain-cleanup-functions.patch @@ -0,0 +1,42 @@ +From 42123e9614ea73c7f64c684c90e4dbb049ef67ef Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Sun, 5 Dec 2021 10:25:28 +0100 +Subject: [PATCH] lgtm: ignore certain cleanup functions + +as they don't do any illegal stuff even when used with an uninitialized +variable. + +(cherry picked from commit af1868213657b38b8d4008608976eb81546cfb8e) + +Related: #2017033 +--- + .lgtm/cpp-queries/UninitializedVariableWithCleanup.ql | 9 +++++++++ + 1 file changed, 9 insertions(+) + +diff --git a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql +index 6bf0ae01eb..8c24b6d8f1 100644 +--- a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql ++++ b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql +@@ -34,6 +34,13 @@ predicate allocatedType(Type t) { + allocatedType(t.getUnspecifiedType()) + } + ++/** Auxiliary predicate: List cleanup functions we want to explicitly ignore ++ * since they don't do anything illegal even when the variable is uninitialized ++ */ ++predicate cleanupFunctionDenyList(string fun) { ++ fun = "erase_char" ++} ++ + /** + * A declaration of a local variable using __attribute__((__cleanup__(x))) + * that leaves the variable uninitialized. +@@ -43,6 +50,8 @@ DeclStmt declWithNoInit(LocalVariable v) { + not exists(v.getInitializer()) and + /* The variable has __attribute__((__cleanup__(...))) set */ + v.getAnAttribute().hasName("cleanup") and ++ /* Check if the cleanup function is not on a deny list */ ++ not exists(Attribute a | a = v.getAnAttribute() and a.getName() = "cleanup" | cleanupFunctionDenyList(a.getAnArgument().getValueText())) and + /* The type of the variable is not stack-allocated. */ + exists(Type t | t = v.getType() | not allocatedType(t)) + } diff --git a/SOURCES/0687-lgtm-detect-more-possible-problematic-scenarios.patch b/SOURCES/0687-lgtm-detect-more-possible-problematic-scenarios.patch new file mode 100644 index 0000000..24edd31 --- /dev/null +++ b/SOURCES/0687-lgtm-detect-more-possible-problematic-scenarios.patch @@ -0,0 +1,96 @@ +From f9b19c9d4caaf870b30cce8a3d6be79eda099c4e Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Sun, 5 Dec 2021 16:11:35 +0100 +Subject: [PATCH] lgtm: detect more possible problematic scenarios + +1) don't ignore stack-allocated variables, since they may hide + heap-allocated stuff (compound types) +2) check if there's a return between the variable declaration and its + initialization; if so, treat the variable as uninitialized +3) introduction of 2) increased the query runtime exponentially, so + introduce some optimizations to bring it back to some reasonable + values + +(cherry picked from commit c8fec8bf9b086f9fc7638db0f1a613a00d7c63a3) + +Related: #2017033 +--- + .../UninitializedVariableWithCleanup.ql | 48 ++++++++++--------- + 1 file changed, 25 insertions(+), 23 deletions(-) + +diff --git a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql +index 8c24b6d8f1..6b3b62f8bc 100644 +--- a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql ++++ b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql +@@ -16,24 +16,6 @@ + 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()) +-} +- + /** Auxiliary predicate: List cleanup functions we want to explicitly ignore + * since they don't do anything illegal even when the variable is uninitialized + */ +@@ -47,13 +29,11 @@ predicate cleanupFunctionDenyList(string fun) { + */ + DeclStmt declWithNoInit(LocalVariable v) { + result.getADeclaration() = v and +- not exists(v.getInitializer()) and ++ not v.hasInitializer() and + /* The variable has __attribute__((__cleanup__(...))) set */ + v.getAnAttribute().hasName("cleanup") and + /* Check if the cleanup function is not on a deny list */ +- not exists(Attribute a | a = v.getAnAttribute() and a.getName() = "cleanup" | cleanupFunctionDenyList(a.getAnArgument().getValueText())) and +- /* The type of the variable is not stack-allocated. */ +- exists(Type t | t = v.getType() | not allocatedType(t)) ++ not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText()) + } + + class UninitialisedLocalReachability extends StackVariableReachability { +@@ -78,7 +58,29 @@ class UninitialisedLocalReachability extends StackVariableReachability { + override predicate isBarrier(ControlFlowNode node, StackVariable v) { + // only report the _first_ possibly uninitialized use + useOfVar(v, node) or +- definitionBarrier(v, node) ++ ( ++ /* If there's an return statement somewhere between the variable declaration ++ * and a possible definition, don't accept is as a valid initialization. ++ * ++ * E.g.: ++ * _cleanup_free_ char *x; ++ * ... ++ * if (...) ++ * return; ++ * ... ++ * x = malloc(...); ++ * ++ * is not a valid initialization, since we might return from the function ++ * _before_ the actual iniitialization (emphasis on _might_, since we ++ * don't know if the return statement might ever evaluate to true). ++ */ ++ definitionBarrier(v, node) and ++ not exists(ReturnStmt rs | ++ /* The attribute check is "just" a complexity optimization */ ++ v.getFunction() = rs.getEnclosingFunction() and v.getAnAttribute().hasName("cleanup") | ++ rs.getLocation().isBefore(node.getLocation()) ++ ) ++ ) + } + } + diff --git a/SOURCES/0688-lgtm-enable-more-and-potentially-useful-queries.patch b/SOURCES/0688-lgtm-enable-more-and-potentially-useful-queries.patch new file mode 100644 index 0000000..784850e --- /dev/null +++ b/SOURCES/0688-lgtm-enable-more-and-potentially-useful-queries.patch @@ -0,0 +1,48 @@ +From 842c676a36abab0d92f1e68de2c8881fd00fdf4b Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Tue, 30 Nov 2021 23:40:28 +0100 +Subject: [PATCH] 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. + +(cherry picked from commit 38f36b9f3443b4d2085799c772e901a402b84af3) + +Related: #2017033 +--- + .lgtm.yml | 24 ++++++++++++++++++++++++ + 1 file changed, 24 insertions(+) + +diff --git a/.lgtm.yml b/.lgtm.yml +index 5948d8c2bc..fe93957b67 100644 +--- a/.lgtm.yml ++++ b/.lgtm.yml +@@ -1,3 +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: diff --git a/SOURCES/0689-meson-avoid-bogus-meson-warning.patch b/SOURCES/0689-meson-avoid-bogus-meson-warning.patch new file mode 100644 index 0000000..3453c1f --- /dev/null +++ b/SOURCES/0689-meson-avoid-bogus-meson-warning.patch @@ -0,0 +1,36 @@ +From 4433c31a80c4477b0a0c503c74e8faebc44f4453 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= +Date: Thu, 7 Nov 2019 11:32:26 +0100 +Subject: [PATCH] meson: avoid bogus meson warning + +With meson-0.52.0-1.module_f31+6771+f5d842eb.noarch I get: +src/test/meson.build:19: WARNING: Overriding previous value of environment variable 'PATH' with a new one + +When we're using *prepend*, the whole point is to modify an existing variable, +so meson shouldn't warn. But let's set avoid the warning and shorten things by +setting the final value immediately. + +(cherry picked from commit cbe804947482998cc767bfb0c169e6263a6ef097) +--- + src/test/meson.build | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/src/test/meson.build b/src/test/meson.build +index 40cf56d73d..6eaa62e53f 100644 +--- a/src/test/meson.build ++++ b/src/test/meson.build +@@ -10,12 +10,11 @@ test_hashmap_ordered_c = custom_target( + + test_include_dir = include_directories('.') + +-path = run_command('sh', ['-c', 'echo "$PATH"']).stdout() ++path = run_command('sh', ['-c', 'echo "$PATH"']).stdout().strip() + test_env = environment() + test_env.set('SYSTEMD_KBD_MODEL_MAP', kbd_model_map) + test_env.set('SYSTEMD_LANGUAGE_FALLBACK_MAP', language_fallback_map) +-test_env.set('PATH', path) +-test_env.prepend('PATH', meson.build_root()) ++test_env.set('PATH', '@0@:@1@'.format(meson.build_root(), path)) + + ############################################################ + diff --git a/SOURCES/0690-test-make-TEST-47-less-racy.patch b/SOURCES/0690-test-make-TEST-47-less-racy.patch new file mode 100644 index 0000000..4aa0371 --- /dev/null +++ b/SOURCES/0690-test-make-TEST-47-less-racy.patch @@ -0,0 +1,33 @@ +From de7125dcfe6d6c8af05262ab786f9fe7cbf15113 Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Wed, 15 Dec 2021 12:15:19 +0100 +Subject: [PATCH] test: make TEST-47 less racy + +Based on: + - 2e7090e94d0c8b31d418555ab2f6a9b75318f6a4 + - e00e2e0b50bbd120290572c8d1242703fb98b34e + - 197298ff9fc930de450330095cc5b67d165d0801 + +Related: #2017033 +--- + test/TEST-47-ISSUE-14566/testsuite.sh | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/test/TEST-47-ISSUE-14566/testsuite.sh b/test/TEST-47-ISSUE-14566/testsuite.sh +index d917cf52ff..b12b50e96c 100755 +--- a/test/TEST-47-ISSUE-14566/testsuite.sh ++++ b/test/TEST-47-ISSUE-14566/testsuite.sh +@@ -6,11 +6,13 @@ systemd-analyze log-level debug + systemd-analyze log-target console + + systemctl start issue_14566_test ++sleep 4 + systemctl status issue_14566_test + + leaked_pid=$(cat /leakedtestpid) + + systemctl stop issue_14566_test ++sleep 4 + + # Leaked PID will still be around if we're buggy. + # I personally prefer to see 42. diff --git a/SOURCES/0691-core-rename-unit_-start_limit-condition-assert-_test.patch b/SOURCES/0691-core-rename-unit_-start_limit-condition-assert-_test.patch new file mode 100644 index 0000000..2a25449 --- /dev/null +++ b/SOURCES/0691-core-rename-unit_-start_limit-condition-assert-_test.patch @@ -0,0 +1,179 @@ +From 894d307d0d149adb46e630550566e5a3f6ff8d2e Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 18 Mar 2019 12:21:27 +0100 +Subject: [PATCH] core: rename unit_{start_limit|condition|assert}_test() to + unit_test_xyz() + +Just some renaming, no change in behaviour. + +Background: I'd like to add more functions unit_test_xyz() that test +various things, hence let's streamline the naming a bit. + +(cherry picked from commit 97a3f4ee052e1b8a0ff03accfa478e352891a84f) + +Related: #2036608 +--- + src/core/automount.c | 2 +- + src/core/mount.c | 2 +- + src/core/path.c | 2 +- + src/core/service.c | 2 +- + src/core/socket.c | 2 +- + src/core/swap.c | 2 +- + src/core/timer.c | 2 +- + src/core/unit.c | 11 +++++------ + src/core/unit.h | 2 +- + 9 files changed, 13 insertions(+), 14 deletions(-) + +diff --git a/src/core/automount.c b/src/core/automount.c +index 76e70f4dac..2bc160cb07 100644 +--- a/src/core/automount.c ++++ b/src/core/automount.c +@@ -808,7 +808,7 @@ static int automount_start(Unit *u) { + return -ENOENT; + } + +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + automount_enter_dead(a, AUTOMOUNT_FAILURE_START_LIMIT_HIT); + return r; +diff --git a/src/core/mount.c b/src/core/mount.c +index 7e80a0c974..aa586d88cb 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1065,7 +1065,7 @@ static int mount_start(Unit *u) { + + assert(IN_SET(m->state, MOUNT_DEAD, MOUNT_FAILED)); + +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT); + return r; +diff --git a/src/core/path.c b/src/core/path.c +index ed40bc6c19..4bccc0396b 100644 +--- a/src/core/path.c ++++ b/src/core/path.c +@@ -565,7 +565,7 @@ static int path_start(Unit *u) { + return -ENOENT; + } + +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + path_enter_dead(p, PATH_FAILURE_START_LIMIT_HIT); + return r; +diff --git a/src/core/service.c b/src/core/service.c +index 7969bbf071..1a1de43d0d 100644 +--- a/src/core/service.c ++++ b/src/core/service.c +@@ -2388,7 +2388,7 @@ static int service_start(Unit *u) { + assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)); + + /* Make sure we don't enter a busy loop of some kind. */ +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + service_enter_dead(s, SERVICE_FAILURE_START_LIMIT_HIT, false); + return r; +diff --git a/src/core/socket.c b/src/core/socket.c +index 50c32ed8f4..09491c6677 100644 +--- a/src/core/socket.c ++++ b/src/core/socket.c +@@ -2469,7 +2469,7 @@ static int socket_start(Unit *u) { + + assert(IN_SET(s->state, SOCKET_DEAD, SOCKET_FAILED)); + +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + socket_enter_dead(s, SOCKET_FAILURE_START_LIMIT_HIT); + return r; +diff --git a/src/core/swap.c b/src/core/swap.c +index a8f127f660..823699699e 100644 +--- a/src/core/swap.c ++++ b/src/core/swap.c +@@ -851,7 +851,7 @@ static int swap_start(Unit *u) { + if (UNIT(other)->job && UNIT(other)->job->state == JOB_RUNNING) + return -EAGAIN; + +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + swap_enter_dead(s, SWAP_FAILURE_START_LIMIT_HIT); + return r; +diff --git a/src/core/timer.c b/src/core/timer.c +index 1718ffc5a5..be16321296 100644 +--- a/src/core/timer.c ++++ b/src/core/timer.c +@@ -599,7 +599,7 @@ static int timer_start(Unit *u) { + return -ENOENT; + } + +- r = unit_start_limit_test(u); ++ r = unit_test_start_limit(u); + if (r < 0) { + timer_enter_dead(t, TIMER_FAILURE_START_LIMIT_HIT); + return r; +diff --git a/src/core/unit.c b/src/core/unit.c +index 23afa24c77..9013186d8a 100644 +--- a/src/core/unit.c ++++ b/src/core/unit.c +@@ -1633,7 +1633,7 @@ static bool unit_condition_test_list(Unit *u, Condition *first, const char *(*to + return triggered != 0; + } + +-static bool unit_condition_test(Unit *u) { ++static bool unit_test_condition(Unit *u) { + assert(u); + + dual_timestamp_get(&u->condition_timestamp); +@@ -1642,7 +1642,7 @@ static bool unit_condition_test(Unit *u) { + return u->condition_result; + } + +-static bool unit_assert_test(Unit *u) { ++static bool unit_test_assert(Unit *u) { + assert(u); + + dual_timestamp_get(&u->assert_timestamp); +@@ -1657,8 +1657,7 @@ void unit_status_printf(Unit *u, const char *status, const char *unit_status_msg + REENABLE_WARNING; + } + +- +-int unit_start_limit_test(Unit *u) { ++int unit_test_start_limit(Unit *u) { + assert(u); + + if (ratelimit_below(&u->start_limit)) { +@@ -1750,14 +1749,14 @@ int unit_start(Unit *u) { + * speed up activation in case there is some hold-off time, + * but we don't want to recheck the condition in that case. */ + if (state != UNIT_ACTIVATING && +- !unit_condition_test(u)) { ++ !unit_test_condition(u)) { + log_unit_debug(u, "Starting requested but condition failed. Not starting unit."); + return -EALREADY; + } + + /* If the asserts failed, fail the entire job */ + if (state != UNIT_ACTIVATING && +- !unit_assert_test(u)) { ++ !unit_test_assert(u)) { + log_unit_notice(u, "Starting requested but asserts failed."); + return -EPROTO; + } +diff --git a/src/core/unit.h b/src/core/unit.h +index ec45b5fb48..a8bc350b66 100644 +--- a/src/core/unit.h ++++ b/src/core/unit.h +@@ -786,7 +786,7 @@ static inline bool unit_supported(Unit *u) { + void unit_warn_if_dir_nonempty(Unit *u, const char* where); + int unit_fail_if_noncanonical(Unit *u, const char* where); + +-int unit_start_limit_test(Unit *u); ++int unit_test_start_limit(Unit *u); + + void unit_unref_uid(Unit *u, bool destroy_now); + int unit_ref_uid(Unit *u, uid_t uid, bool clean_ipc); diff --git a/SOURCES/0692-core-Check-unit-start-rate-limiting-earlier.patch b/SOURCES/0692-core-Check-unit-start-rate-limiting-earlier.patch new file mode 100644 index 0000000..cbe1aac --- /dev/null +++ b/SOURCES/0692-core-Check-unit-start-rate-limiting-earlier.patch @@ -0,0 +1,400 @@ +From 471eda89a25a3ceac91a2d05e39a54aae78038ed Mon Sep 17 00:00:00 2001 +From: Daan De Meyer +Date: Tue, 24 Aug 2021 16:46:47 +0100 +Subject: [PATCH] core: Check unit start rate limiting earlier + +Fixes #17433. Currently, if any of the validations we do before we +check start rate limiting fail, we can still enter a busy loop as +no rate limiting gets applied. A common occurence of this scenario +is path units triggering a service that fails a condition check. + +To fix the issue, we simply move up start rate limiting checks to +be the first thing we do when starting a unit. To achieve this, +we add a new method to the unit vtable and implement it for the +relevant unit types so that we can do the start rate limit checks +earlier on. + +(cherry picked from commit 9727f2427ff6b2e1f4ab927cc57ad8e888f04e95) + +Related: #2036608 + +[msekleta: I've deleted part of the original commit that adds test for +issue #17433. This was necessary because upstream commit assumes newer +organization of the test code which we don't have in RHEL-8 tree. As +a consequence we must add explicit test for this in the internal +test-suite.] +--- + src/core/automount.c | 23 +++++++++++++++++------ + src/core/mount.c | 23 +++++++++++++++++------ + src/core/path.c | 23 +++++++++++++++++------ + src/core/service.c | 25 ++++++++++++++++++------- + src/core/socket.c | 23 +++++++++++++++++------ + src/core/swap.c | 23 +++++++++++++++++------ + src/core/timer.c | 22 ++++++++++++++++------ + src/core/unit.c | 14 ++++++++++---- + src/core/unit.h | 4 ++++ + 9 files changed, 133 insertions(+), 47 deletions(-) + +diff --git a/src/core/automount.c b/src/core/automount.c +index 2bc160cb07..5e16adabb5 100644 +--- a/src/core/automount.c ++++ b/src/core/automount.c +@@ -808,12 +808,6 @@ static int automount_start(Unit *u) { + return -ENOENT; + } + +- r = unit_test_start_limit(u); +- if (r < 0) { +- automount_enter_dead(a, AUTOMOUNT_FAILURE_START_LIMIT_HIT); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -1077,6 +1071,21 @@ static bool automount_supported(void) { + return supported; + } + ++static int automount_test_start_limit(Unit *u) { ++ Automount *a = AUTOMOUNT(u); ++ int r; ++ ++ assert(a); ++ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ automount_enter_dead(a, AUTOMOUNT_FAILURE_START_LIMIT_HIT); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const automount_result_table[_AUTOMOUNT_RESULT_MAX] = { + [AUTOMOUNT_SUCCESS] = "success", + [AUTOMOUNT_FAILURE_RESOURCES] = "resources", +@@ -1135,4 +1144,6 @@ const UnitVTable automount_vtable = { + [JOB_FAILED] = "Failed to unset automount %s.", + }, + }, ++ ++ .test_start_limit = automount_test_start_limit, + }; +diff --git a/src/core/mount.c b/src/core/mount.c +index aa586d88cb..22848847e5 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1065,12 +1065,6 @@ static int mount_start(Unit *u) { + + assert(IN_SET(m->state, MOUNT_DEAD, MOUNT_FAILED)); + +- r = unit_test_start_limit(u); +- if (r < 0) { +- mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -1957,6 +1951,21 @@ static int mount_control_pid(Unit *u) { + return m->control_pid; + } + ++static int mount_test_start_limit(Unit *u) { ++ Mount *m = MOUNT(u); ++ int r; ++ ++ assert(m); ++ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const mount_exec_command_table[_MOUNT_EXEC_COMMAND_MAX] = { + [MOUNT_EXEC_MOUNT] = "ExecMount", + [MOUNT_EXEC_UNMOUNT] = "ExecUnmount", +@@ -2048,4 +2057,6 @@ const UnitVTable mount_vtable = { + [JOB_TIMEOUT] = "Timed out unmounting %s.", + }, + }, ++ ++ .test_start_limit = mount_test_start_limit, + }; +diff --git a/src/core/path.c b/src/core/path.c +index 4bccc0396b..1e69a1f05f 100644 +--- a/src/core/path.c ++++ b/src/core/path.c +@@ -565,12 +565,6 @@ static int path_start(Unit *u) { + return -ENOENT; + } + +- r = unit_test_start_limit(u); +- if (r < 0) { +- path_enter_dead(p, PATH_FAILURE_START_LIMIT_HIT); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -730,6 +724,21 @@ static void path_reset_failed(Unit *u) { + p->result = PATH_SUCCESS; + } + ++static int path_test_start_limit(Unit *u) { ++ Path *p = PATH(u); ++ int r; ++ ++ assert(p); ++ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ path_enter_dead(p, PATH_FAILURE_START_LIMIT_HIT); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const path_type_table[_PATH_TYPE_MAX] = { + [PATH_EXISTS] = "PathExists", + [PATH_EXISTS_GLOB] = "PathExistsGlob", +@@ -782,4 +791,6 @@ const UnitVTable path_vtable = { + + .bus_vtable = bus_path_vtable, + .bus_set_property = bus_path_set_property, ++ ++ .test_start_limit = path_test_start_limit, + }; +diff --git a/src/core/service.c b/src/core/service.c +index 1a1de43d0d..c5f408d817 100644 +--- a/src/core/service.c ++++ b/src/core/service.c +@@ -2387,13 +2387,6 @@ static int service_start(Unit *u) { + + assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)); + +- /* Make sure we don't enter a busy loop of some kind. */ +- r = unit_test_start_limit(u); +- if (r < 0) { +- service_enter_dead(s, SERVICE_FAILURE_START_LIMIT_HIT, false); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -4081,6 +4074,22 @@ static bool service_needs_console(Unit *u) { + SERVICE_FINAL_SIGKILL); + } + ++static int service_test_start_limit(Unit *u) { ++ Service *s = SERVICE(u); ++ int r; ++ ++ assert(s); ++ ++ /* Make sure we don't enter a busy loop of some kind. */ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ service_enter_dead(s, SERVICE_FAILURE_START_LIMIT_HIT, false); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const service_restart_table[_SERVICE_RESTART_MAX] = { + [SERVICE_RESTART_NO] = "no", + [SERVICE_RESTART_ON_SUCCESS] = "on-success", +@@ -4222,4 +4231,6 @@ const UnitVTable service_vtable = { + [JOB_FAILED] = "Stopped (with error) %s.", + }, + }, ++ ++ .test_start_limit = service_test_start_limit, + }; +diff --git a/src/core/socket.c b/src/core/socket.c +index 09491c6677..36d2e4f823 100644 +--- a/src/core/socket.c ++++ b/src/core/socket.c +@@ -2469,12 +2469,6 @@ static int socket_start(Unit *u) { + + assert(IN_SET(s->state, SOCKET_DEAD, SOCKET_FAILED)); + +- r = unit_test_start_limit(u); +- if (r < 0) { +- socket_enter_dead(s, SOCKET_FAILURE_START_LIMIT_HIT); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -3267,6 +3261,21 @@ static int socket_control_pid(Unit *u) { + return s->control_pid; + } + ++static int socket_test_start_limit(Unit *u) { ++ Socket *s = SOCKET(u); ++ int r; ++ ++ assert(s); ++ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ socket_enter_dead(s, SOCKET_FAILURE_START_LIMIT_HIT); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const socket_exec_command_table[_SOCKET_EXEC_COMMAND_MAX] = { + [SOCKET_EXEC_START_PRE] = "ExecStartPre", + [SOCKET_EXEC_START_CHOWN] = "ExecStartChown", +@@ -3359,4 +3368,6 @@ const UnitVTable socket_vtable = { + [JOB_TIMEOUT] = "Timed out stopping %s.", + }, + }, ++ ++ .test_start_limit = socket_test_start_limit, + }; +diff --git a/src/core/swap.c b/src/core/swap.c +index 823699699e..90fcd69300 100644 +--- a/src/core/swap.c ++++ b/src/core/swap.c +@@ -851,12 +851,6 @@ static int swap_start(Unit *u) { + if (UNIT(other)->job && UNIT(other)->job->state == JOB_RUNNING) + return -EAGAIN; + +- r = unit_test_start_limit(u); +- if (r < 0) { +- swap_enter_dead(s, SWAP_FAILURE_START_LIMIT_HIT); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -1458,6 +1452,21 @@ static int swap_control_pid(Unit *u) { + return s->control_pid; + } + ++static int swap_test_start_limit(Unit *u) { ++ Swap *s = SWAP(u); ++ int r; ++ ++ assert(s); ++ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ swap_enter_dead(s, SWAP_FAILURE_START_LIMIT_HIT); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const swap_exec_command_table[_SWAP_EXEC_COMMAND_MAX] = { + [SWAP_EXEC_ACTIVATE] = "ExecActivate", + [SWAP_EXEC_DEACTIVATE] = "ExecDeactivate", +@@ -1547,4 +1556,6 @@ const UnitVTable swap_vtable = { + [JOB_TIMEOUT] = "Timed out deactivating swap %s.", + }, + }, ++ ++ .test_start_limit = swap_test_start_limit, + }; +diff --git a/src/core/timer.c b/src/core/timer.c +index be16321296..fb9ae17990 100644 +--- a/src/core/timer.c ++++ b/src/core/timer.c +@@ -599,12 +599,6 @@ static int timer_start(Unit *u) { + return -ENOENT; + } + +- r = unit_test_start_limit(u); +- if (r < 0) { +- timer_enter_dead(t, TIMER_FAILURE_START_LIMIT_HIT); +- return r; +- } +- + r = unit_acquire_invocation_id(u); + if (r < 0) + return r; +@@ -829,6 +823,21 @@ static void timer_timezone_change(Unit *u) { + timer_enter_waiting(t, false); + } + ++static int timer_test_start_limit(Unit *u) { ++ Timer *t = TIMER(u); ++ int r; ++ ++ assert(t); ++ ++ r = unit_test_start_limit(u); ++ if (r < 0) { ++ timer_enter_dead(t, TIMER_FAILURE_START_LIMIT_HIT); ++ return r; ++ } ++ ++ return 0; ++} ++ + static const char* const timer_base_table[_TIMER_BASE_MAX] = { + [TIMER_ACTIVE] = "OnActiveSec", + [TIMER_BOOT] = "OnBootSec", +@@ -884,4 +893,5 @@ const UnitVTable timer_vtable = { + .bus_set_property = bus_timer_set_property, + + .can_transient = true, ++ .test_start_limit = timer_test_start_limit, + }; +diff --git a/src/core/unit.c b/src/core/unit.c +index 9013186d8a..f0df7452fa 100644 +--- a/src/core/unit.c ++++ b/src/core/unit.c +@@ -1728,10 +1728,16 @@ int unit_start(Unit *u) { + + assert(u); + +- /* If this is already started, then this will succeed. Note +- * that this will even succeed if this unit is not startable +- * by the user. This is relied on to detect when we need to +- * wait for units and when waiting is finished. */ ++ /* Check start rate limiting early so that failure conditions don't cause us to enter a busy loop. */ ++ if (UNIT_VTABLE(u)->test_start_limit) { ++ int r = UNIT_VTABLE(u)->test_start_limit(u); ++ if (r < 0) ++ return r; ++ } ++ ++ /* If this is already started, then this will succeed. Note that this will even succeed if this unit ++ * is not startable by the user. This is relied on to detect when we need to wait for units and when ++ * waiting is finished. */ + state = unit_active_state(u); + if (UNIT_IS_ACTIVE_OR_RELOADING(state)) + return -EALREADY; +diff --git a/src/core/unit.h b/src/core/unit.h +index a8bc350b66..9e6f1bcf81 100644 +--- a/src/core/unit.h ++++ b/src/core/unit.h +@@ -567,6 +567,10 @@ typedef struct UnitVTable { + /* The bus vtable */ + const sd_bus_vtable *bus_vtable; + ++ /* If this function is set, it's invoked first as part of starting a unit to allow start rate ++ * limiting checks to occur before we do anything else. */ ++ int (*test_start_limit)(Unit *u); ++ + /* The strings to print in status messages */ + UnitStatusMessageFormats status_message_formats; + diff --git a/SOURCES/0693-sd-event-introduce-callback-invoked-when-event-sourc.patch b/SOURCES/0693-sd-event-introduce-callback-invoked-when-event-sourc.patch new file mode 100644 index 0000000..42d59d7 --- /dev/null +++ b/SOURCES/0693-sd-event-introduce-callback-invoked-when-event-sourc.patch @@ -0,0 +1,226 @@ +From 51210a849ea7f163a1760de989756206c01dd758 Mon Sep 17 00:00:00 2001 +From: Michal Sekletar +Date: Mon, 4 Oct 2021 19:44:06 +0200 +Subject: [PATCH] sd-event: introduce callback invoked when event source + ratelimit expires + +(cherry picked from commit fd69f2247520b0be3190ded96d646a415edc97b7) + +Related: #2036608 +--- + src/libsystemd/libsystemd.sym | 5 +++ + src/libsystemd/sd-event/sd-event.c | 61 +++++++++++++++++++++++----- + src/libsystemd/sd-event/test-event.c | 12 ++++++ + src/systemd/sd-event.h | 1 + + 4 files changed, 68 insertions(+), 11 deletions(-) + +diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym +index 149d2e7b82..f4a1426248 100644 +--- a/src/libsystemd/libsystemd.sym ++++ b/src/libsystemd/libsystemd.sym +@@ -579,3 +579,8 @@ global: + sd_event_source_get_ratelimit; + sd_event_source_is_ratelimited; + } LIBSYSTEMD_239; ++ ++LIBSYSTEMD_250 { ++global: ++ sd_event_source_set_ratelimit_expire_callback; ++} LIBSYSTEMD_248; +diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c +index 47cf93b3f4..0adfdd9e1a 100644 +--- a/src/libsystemd/sd-event/sd-event.c ++++ b/src/libsystemd/sd-event/sd-event.c +@@ -125,6 +125,7 @@ struct sd_event_source { + uint64_t prepare_iteration; + + sd_event_destroy_t destroy_callback; ++ sd_event_handler_t ratelimit_expire_callback; + + LIST_FIELDS(sd_event_source, sources); + +@@ -2734,7 +2735,7 @@ fail: + return r; + } + +-static int event_source_leave_ratelimit(sd_event_source *s) { ++static int event_source_leave_ratelimit(sd_event_source *s, bool run_callback) { + int r; + + assert(s); +@@ -2766,6 +2767,23 @@ static int event_source_leave_ratelimit(sd_event_source *s) { + ratelimit_reset(&s->rate_limit); + + log_debug("Event source %p (%s) left rate limit state.", s, strna(s->description)); ++ ++ if (run_callback && s->ratelimit_expire_callback) { ++ s->dispatching = true; ++ r = s->ratelimit_expire_callback(s, s->userdata); ++ s->dispatching = false; ++ ++ if (r < 0) { ++ log_debug_errno(r, "Ratelimit expiry callback of event source %s (type %s) returned error, disabling: %m", ++ strna(s->description), ++ event_source_type_to_string(s->type)); ++ ++ sd_event_source_set_enabled(s, SD_EVENT_OFF); ++ } ++ ++ return 1; ++ } ++ + return 0; + + fail: +@@ -2966,6 +2984,7 @@ static int process_timer( + struct clock_data *d) { + + sd_event_source *s; ++ bool callback_invoked = false; + int r; + + assert(e); +@@ -2981,9 +3000,11 @@ static int process_timer( + * again. */ + assert(s->ratelimited); + +- r = event_source_leave_ratelimit(s); ++ r = event_source_leave_ratelimit(s, /* run_callback */ true); + if (r < 0) + return r; ++ else if (r == 1) ++ callback_invoked = true; + + continue; + } +@@ -2998,7 +3019,7 @@ static int process_timer( + event_source_time_prioq_reshuffle(s); + } + +- return 0; ++ return callback_invoked; + } + + static int process_child(sd_event *e) { +@@ -3698,15 +3719,15 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { + if (r < 0) + goto finish; + +- r = process_timer(e, e->timestamp.realtime, &e->realtime); ++ r = process_inotify(e); + if (r < 0) + goto finish; + +- r = process_timer(e, e->timestamp.boottime, &e->boottime); ++ r = process_timer(e, e->timestamp.realtime, &e->realtime); + if (r < 0) + goto finish; + +- r = process_timer(e, e->timestamp.monotonic, &e->monotonic); ++ r = process_timer(e, e->timestamp.boottime, &e->boottime); + if (r < 0) + goto finish; + +@@ -3718,16 +3739,27 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { + if (r < 0) + goto finish; + ++ r = process_timer(e, e->timestamp.monotonic, &e->monotonic); ++ if (r < 0) ++ goto finish; ++ else if (r == 1) { ++ /* Ratelimit expiry callback was called. Let's postpone processing pending sources and ++ * put loop in the initial state in order to evaluate (in the next iteration) also sources ++ * there were potentially re-enabled by the callback. ++ * ++ * Wondering why we treat only this invocation of process_timer() differently? Once event ++ * source is ratelimited we essentially transform it into CLOCK_MONOTONIC timer hence ++ * ratelimit expiry callback is never called for any other timer type. */ ++ r = 0; ++ goto finish; ++ } ++ + if (e->need_process_child) { + r = process_child(e); + if (r < 0) + goto finish; + } + +- r = process_inotify(e); +- if (r < 0) +- goto finish; +- + if (event_next_pending(e)) { + e->state = SD_EVENT_PENDING; + +@@ -4054,7 +4086,7 @@ _public_ int sd_event_source_set_ratelimit(sd_event_source *s, uint64_t interval + + /* When ratelimiting is configured we'll always reset the rate limit state first and start fresh, + * non-ratelimited. */ +- r = event_source_leave_ratelimit(s); ++ r = event_source_leave_ratelimit(s, /* run_callback */ false); + if (r < 0) + return r; + +@@ -4062,6 +4094,13 @@ _public_ int sd_event_source_set_ratelimit(sd_event_source *s, uint64_t interval + return 0; + } + ++_public_ int sd_event_source_set_ratelimit_expire_callback(sd_event_source *s, sd_event_handler_t callback) { ++ assert_return(s, -EINVAL); ++ ++ s->ratelimit_expire_callback = callback; ++ return 0; ++} ++ + _public_ int sd_event_source_get_ratelimit(sd_event_source *s, uint64_t *ret_interval, unsigned *ret_burst) { + assert_return(s, -EINVAL); + +diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c +index e3ee4cd5c3..9135b22839 100644 +--- a/src/libsystemd/sd-event/test-event.c ++++ b/src/libsystemd/sd-event/test-event.c +@@ -506,6 +506,11 @@ static int ratelimit_time_handler(sd_event_source *s, uint64_t usec, void *userd + return 0; + } + ++static int expired = -1; ++static int ratelimit_expired(sd_event_source *s, void *userdata) { ++ return ++expired; ++} ++ + static void test_ratelimit(void) { + _cleanup_close_pair_ int p[2] = {-1, -1}; + _cleanup_(sd_event_unrefp) sd_event *e = NULL; +@@ -568,12 +573,19 @@ static void test_ratelimit(void) { + + assert_se(sd_event_source_set_ratelimit(s, 1 * USEC_PER_SEC, 10) >= 0); + ++ /* Set callback that will be invoked when we leave rate limited state. */ ++ assert_se(sd_event_source_set_ratelimit_expire_callback(s, ratelimit_expired) >= 0); ++ + do { + assert_se(sd_event_run(e, UINT64_MAX) >= 0); + } while (!sd_event_source_is_ratelimited(s)); + + log_info("ratelimit_time_handler: called 10 more times, event source got ratelimited"); + assert_se(count == 20); ++ ++ /* Dispatch the event loop once more and check that ratelimit expiration callback got called */ ++ assert_se(sd_event_run(e, UINT64_MAX) >= 0); ++ assert_se(expired == 0); + } + + int main(int argc, char *argv[]) { +diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h +index a17a9b3488..c2e9c9614d 100644 +--- a/src/systemd/sd-event.h ++++ b/src/systemd/sd-event.h +@@ -147,6 +147,7 @@ int sd_event_source_get_destroy_callback(sd_event_source *s, sd_event_destroy_t + int sd_event_source_set_ratelimit(sd_event_source *s, uint64_t interval_usec, unsigned burst); + int sd_event_source_get_ratelimit(sd_event_source *s, uint64_t *ret_interval_usec, unsigned *ret_burst); + int sd_event_source_is_ratelimited(sd_event_source *s); ++int sd_event_source_set_ratelimit_expire_callback(sd_event_source *s, sd_event_handler_t callback); + + /* Define helpers so that __attribute__((cleanup(sd_event_unrefp))) and similar may be used. */ + _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_event, sd_event_unref); diff --git a/SOURCES/0694-core-rename-generalize-UNIT-u-test_start_limit-hook.patch b/SOURCES/0694-core-rename-generalize-UNIT-u-test_start_limit-hook.patch new file mode 100644 index 0000000..4353eae --- /dev/null +++ b/SOURCES/0694-core-rename-generalize-UNIT-u-test_start_limit-hook.patch @@ -0,0 +1,263 @@ +From 3674514b7220a136dcfd464c205d41609f0c99a7 Mon Sep 17 00:00:00 2001 +From: Michal Sekletar +Date: Mon, 4 Oct 2021 17:51:52 +0200 +Subject: [PATCH] core: rename/generalize UNIT(u)->test_start_limit() hook + +Up until now the main reason why we didn't proceed with starting the +unit was exceed start limit burst. However, for unit types like mounts +the other reason could be effective ratelimit on /proc/self/mountinfo +event source. That means our mount unit state may not reflect current +kernel state. Hence, we need to attempt to re-run the start job again +after ratelimit on event source expires. + +As we will be introducing another reason than start limit let's rename +the virtual function that implements the check. + +(cherry picked from commit 705578c3b9d794097233aa66010cf67b2a444716) + +Related: #2036608 +--- + src/core/automount.c | 6 +++--- + src/core/mount.c | 6 +++--- + src/core/path.c | 6 +++--- + src/core/service.c | 6 +++--- + src/core/socket.c | 6 +++--- + src/core/swap.c | 6 +++--- + src/core/timer.c | 6 +++--- + src/core/unit.c | 6 +++--- + src/core/unit.h | 2 +- + 9 files changed, 25 insertions(+), 25 deletions(-) + +diff --git a/src/core/automount.c b/src/core/automount.c +index 5e16adabb5..f212620c8f 100644 +--- a/src/core/automount.c ++++ b/src/core/automount.c +@@ -1071,7 +1071,7 @@ static bool automount_supported(void) { + return supported; + } + +-static int automount_test_start_limit(Unit *u) { ++static int automount_can_start(Unit *u) { + Automount *a = AUTOMOUNT(u); + int r; + +@@ -1083,7 +1083,7 @@ static int automount_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const automount_result_table[_AUTOMOUNT_RESULT_MAX] = { +@@ -1145,5 +1145,5 @@ const UnitVTable automount_vtable = { + }, + }, + +- .test_start_limit = automount_test_start_limit, ++ .can_start = automount_can_start, + }; +diff --git a/src/core/mount.c b/src/core/mount.c +index 22848847e5..032a2ca156 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1951,7 +1951,7 @@ static int mount_control_pid(Unit *u) { + return m->control_pid; + } + +-static int mount_test_start_limit(Unit *u) { ++static int mount_can_start(Unit *u) { + Mount *m = MOUNT(u); + int r; + +@@ -1963,7 +1963,7 @@ static int mount_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const mount_exec_command_table[_MOUNT_EXEC_COMMAND_MAX] = { +@@ -2058,5 +2058,5 @@ const UnitVTable mount_vtable = { + }, + }, + +- .test_start_limit = mount_test_start_limit, ++ .can_start = mount_can_start, + }; +diff --git a/src/core/path.c b/src/core/path.c +index 1e69a1f05f..58f490589d 100644 +--- a/src/core/path.c ++++ b/src/core/path.c +@@ -724,7 +724,7 @@ static void path_reset_failed(Unit *u) { + p->result = PATH_SUCCESS; + } + +-static int path_test_start_limit(Unit *u) { ++static int path_can_start(Unit *u) { + Path *p = PATH(u); + int r; + +@@ -736,7 +736,7 @@ static int path_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const path_type_table[_PATH_TYPE_MAX] = { +@@ -792,5 +792,5 @@ const UnitVTable path_vtable = { + .bus_vtable = bus_path_vtable, + .bus_set_property = bus_path_set_property, + +- .test_start_limit = path_test_start_limit, ++ .can_start = path_can_start, + }; +diff --git a/src/core/service.c b/src/core/service.c +index c5f408d817..e8ae1a5772 100644 +--- a/src/core/service.c ++++ b/src/core/service.c +@@ -4074,7 +4074,7 @@ static bool service_needs_console(Unit *u) { + SERVICE_FINAL_SIGKILL); + } + +-static int service_test_start_limit(Unit *u) { ++static int service_can_start(Unit *u) { + Service *s = SERVICE(u); + int r; + +@@ -4087,7 +4087,7 @@ static int service_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const service_restart_table[_SERVICE_RESTART_MAX] = { +@@ -4232,5 +4232,5 @@ const UnitVTable service_vtable = { + }, + }, + +- .test_start_limit = service_test_start_limit, ++ .can_start = service_can_start, + }; +diff --git a/src/core/socket.c b/src/core/socket.c +index 36d2e4f823..3589300e68 100644 +--- a/src/core/socket.c ++++ b/src/core/socket.c +@@ -3261,7 +3261,7 @@ static int socket_control_pid(Unit *u) { + return s->control_pid; + } + +-static int socket_test_start_limit(Unit *u) { ++static int socket_can_start(Unit *u) { + Socket *s = SOCKET(u); + int r; + +@@ -3273,7 +3273,7 @@ static int socket_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const socket_exec_command_table[_SOCKET_EXEC_COMMAND_MAX] = { +@@ -3369,5 +3369,5 @@ const UnitVTable socket_vtable = { + }, + }, + +- .test_start_limit = socket_test_start_limit, ++ .can_start = socket_can_start, + }; +diff --git a/src/core/swap.c b/src/core/swap.c +index 90fcd69300..498c5a6d69 100644 +--- a/src/core/swap.c ++++ b/src/core/swap.c +@@ -1452,7 +1452,7 @@ static int swap_control_pid(Unit *u) { + return s->control_pid; + } + +-static int swap_test_start_limit(Unit *u) { ++static int swap_can_start(Unit *u) { + Swap *s = SWAP(u); + int r; + +@@ -1464,7 +1464,7 @@ static int swap_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const swap_exec_command_table[_SWAP_EXEC_COMMAND_MAX] = { +@@ -1557,5 +1557,5 @@ const UnitVTable swap_vtable = { + }, + }, + +- .test_start_limit = swap_test_start_limit, ++ .can_start = swap_can_start, + }; +diff --git a/src/core/timer.c b/src/core/timer.c +index fb9ae17990..684180bf99 100644 +--- a/src/core/timer.c ++++ b/src/core/timer.c +@@ -823,7 +823,7 @@ static void timer_timezone_change(Unit *u) { + timer_enter_waiting(t, false); + } + +-static int timer_test_start_limit(Unit *u) { ++static int timer_can_start(Unit *u) { + Timer *t = TIMER(u); + int r; + +@@ -835,7 +835,7 @@ static int timer_test_start_limit(Unit *u) { + return r; + } + +- return 0; ++ return 1; + } + + static const char* const timer_base_table[_TIMER_BASE_MAX] = { +@@ -893,5 +893,5 @@ const UnitVTable timer_vtable = { + .bus_set_property = bus_timer_set_property, + + .can_transient = true, +- .test_start_limit = timer_test_start_limit, ++ .can_start = timer_can_start, + }; +diff --git a/src/core/unit.c b/src/core/unit.c +index f0df7452fa..4de218feac 100644 +--- a/src/core/unit.c ++++ b/src/core/unit.c +@@ -1728,9 +1728,9 @@ int unit_start(Unit *u) { + + assert(u); + +- /* Check start rate limiting early so that failure conditions don't cause us to enter a busy loop. */ +- if (UNIT_VTABLE(u)->test_start_limit) { +- int r = UNIT_VTABLE(u)->test_start_limit(u); ++ /* Check our ability to start early so that failure conditions don't cause us to enter a busy loop. */ ++ if (UNIT_VTABLE(u)->can_start) { ++ int r = UNIT_VTABLE(u)->can_start(u); + if (r < 0) + return r; + } +diff --git a/src/core/unit.h b/src/core/unit.h +index 9e6f1bcf81..0cd259411f 100644 +--- a/src/core/unit.h ++++ b/src/core/unit.h +@@ -569,7 +569,7 @@ typedef struct UnitVTable { + + /* If this function is set, it's invoked first as part of starting a unit to allow start rate + * limiting checks to occur before we do anything else. */ +- int (*test_start_limit)(Unit *u); ++ int (*can_start)(Unit *u); + + /* The strings to print in status messages */ + UnitStatusMessageFormats status_message_formats; diff --git a/SOURCES/0695-mount-make-mount-units-start-jobs-not-runnable-if-p-.patch b/SOURCES/0695-mount-make-mount-units-start-jobs-not-runnable-if-p-.patch new file mode 100644 index 0000000..cb7a006 --- /dev/null +++ b/SOURCES/0695-mount-make-mount-units-start-jobs-not-runnable-if-p-.patch @@ -0,0 +1,27 @@ +From cb519c7d769851ee5e24c797fc04eaa13383c674 Mon Sep 17 00:00:00 2001 +From: Michal Sekletar +Date: Mon, 4 Oct 2021 19:41:34 +0200 +Subject: [PATCH] mount: make mount units start jobs not runnable if + /p/s/mountinfo ratelimit is in effect + +(cherry picked from commit a7c93dfe91e88a5a561341c523a45c7f8d71a588) + +Related: #2036608 +--- + src/core/mount.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/core/mount.c b/src/core/mount.c +index 032a2ca156..ab09e6fbb0 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1957,6 +1957,9 @@ static int mount_can_start(Unit *u) { + + assert(m); + ++ if (sd_event_source_is_ratelimited(u->manager->mount_event_source)) ++ return -EAGAIN; ++ + r = unit_test_start_limit(u); + if (r < 0) { + mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT); diff --git a/SOURCES/0696-mount-retrigger-run-queue-after-ratelimit-expired-to.patch b/SOURCES/0696-mount-retrigger-run-queue-after-ratelimit-expired-to.patch new file mode 100644 index 0000000..c4909f6 --- /dev/null +++ b/SOURCES/0696-mount-retrigger-run-queue-after-ratelimit-expired-to.patch @@ -0,0 +1,54 @@ +From b0c226e9fd3e6bfa5388832cc2745d9ec935f3ec Mon Sep 17 00:00:00 2001 +From: Michal Sekletar +Date: Mon, 4 Oct 2021 20:31:49 +0200 +Subject: [PATCH] mount: retrigger run queue after ratelimit expired to run + delayed mount start jobs + +Fixes #20329 + +(cherry picked from commit edc027b4f1cfaa49e8ecdde763eb8c623402d464) + +Related: #2036608 +--- + src/core/mount.c | 21 +++++++++++++++++++++ + 1 file changed, 21 insertions(+) + +diff --git a/src/core/mount.c b/src/core/mount.c +index ab09e6fbb0..bdba9e6884 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1710,6 +1710,21 @@ static bool mount_is_mounted(Mount *m) { + return UNIT(m)->perpetual || m->is_mounted; + } + ++static int mount_on_ratelimit_expire(sd_event_source *s, void *userdata) { ++ Manager *m = userdata; ++ int r; ++ ++ assert(m); ++ ++ /* By entering ratelimited state we made all mount start jobs not runnable, now rate limit is over so let's ++ * make sure we dispatch them in the next iteration. */ ++ r = sd_event_source_set_enabled(m->run_queue_event_source, SD_EVENT_ONESHOT); ++ if (r < 0) ++ log_debug_errno(r, "Failed to enable run queue event source, ignoring: %m"); ++ ++ return 0; ++} ++ + static void mount_enumerate(Manager *m) { + int r; + +@@ -1763,6 +1778,12 @@ static void mount_enumerate(Manager *m) { + goto fail; + } + ++ r = sd_event_source_set_ratelimit_expire_callback(m->mount_event_source, mount_on_ratelimit_expire); ++ if (r < 0) { ++ log_error_errno(r, "Failed to enable rate limit for mount events: %m"); ++ goto fail; ++ } ++ + (void) sd_event_source_set_description(m->mount_event_source, "mount-monitor-dispatch"); + } + diff --git a/SOURCES/0697-pid1-add-a-manager_trigger_run_queue-helper.patch b/SOURCES/0697-pid1-add-a-manager_trigger_run_queue-helper.patch new file mode 100644 index 0000000..96b3351 --- /dev/null +++ b/SOURCES/0697-pid1-add-a-manager_trigger_run_queue-helper.patch @@ -0,0 +1,98 @@ +From 5a218b6820be7ffaf21cd42cd4c96b47d18442ee Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Fri, 12 Nov 2021 09:43:07 +0100 +Subject: [PATCH] pid1: add a manager_trigger_run_queue() helper + +We have two different places where we re-trigger the run queue now. +let's unify it under a common function, that is part of the Manager +code. + +Follow-up for #20953 + +(cherry picked from commit b0c4b2824693fe6a27ea9439ec7a6328a0e23704) + +Related: #2036608 +--- + src/core/job.c | 5 ++--- + src/core/manager.c | 12 ++++++++++++ + src/core/manager.h | 2 ++ + src/core/mount.c | 9 +++------ + 4 files changed, 19 insertions(+), 9 deletions(-) + +diff --git a/src/core/job.c b/src/core/job.c +index 43ab55ed18..55f36b928f 100644 +--- a/src/core/job.c ++++ b/src/core/job.c +@@ -1139,11 +1139,10 @@ void job_add_to_run_queue(Job *j) { + if (j->in_run_queue) + return; + +- if (!j->manager->run_queue) +- sd_event_source_set_enabled(j->manager->run_queue_event_source, SD_EVENT_ONESHOT); +- + LIST_PREPEND(run_queue, j->manager->run_queue, j); + j->in_run_queue = true; ++ ++ manager_trigger_run_queue(j->manager); + } + + void job_add_to_dbus_queue(Job *j) { +diff --git a/src/core/manager.c b/src/core/manager.c +index ee976f70b3..845c26f498 100644 +--- a/src/core/manager.c ++++ b/src/core/manager.c +@@ -2120,6 +2120,18 @@ static int manager_dispatch_run_queue(sd_event_source *source, void *userdata) { + return 1; + } + ++void manager_trigger_run_queue(Manager *m) { ++ int r; ++ ++ assert(m); ++ ++ r = sd_event_source_set_enabled( ++ m->run_queue_event_source, ++ m->run_queue ? SD_EVENT_ONESHOT: SD_EVENT_OFF); ++ if (r < 0) ++ log_warning_errno(r, "Failed to enable job run queue event source, ignoring: %m"); ++} ++ + static unsigned manager_dispatch_dbus_queue(Manager *m) { + unsigned n = 0, budget; + Unit *u; +diff --git a/src/core/manager.h b/src/core/manager.h +index c4b8e80093..7b572c8dfd 100644 +--- a/src/core/manager.h ++++ b/src/core/manager.h +@@ -416,6 +416,8 @@ unsigned manager_dispatch_load_queue(Manager *m); + int manager_environment_add(Manager *m, char **minus, char **plus); + int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit); + ++void manager_trigger_run_queue(Manager *m); ++ + int manager_loop(Manager *m); + + int manager_open_serialization(Manager *m, FILE **_f); +diff --git a/src/core/mount.c b/src/core/mount.c +index bdba9e6884..c17154cde1 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1712,15 +1712,12 @@ static bool mount_is_mounted(Mount *m) { + + static int mount_on_ratelimit_expire(sd_event_source *s, void *userdata) { + Manager *m = userdata; +- int r; + + assert(m); + +- /* By entering ratelimited state we made all mount start jobs not runnable, now rate limit is over so let's +- * make sure we dispatch them in the next iteration. */ +- r = sd_event_source_set_enabled(m->run_queue_event_source, SD_EVENT_ONESHOT); +- if (r < 0) +- log_debug_errno(r, "Failed to enable run queue event source, ignoring: %m"); ++ /* By entering ratelimited state we made all mount start jobs not runnable, now rate limit is over so ++ * let's make sure we dispatch them in the next iteration. */ ++ manager_trigger_run_queue(m); + + return 0; + } diff --git a/SOURCES/0698-unit-add-jobs-that-were-skipped-because-of-ratelimit.patch b/SOURCES/0698-unit-add-jobs-that-were-skipped-because-of-ratelimit.patch new file mode 100644 index 0000000..bc7fe06 --- /dev/null +++ b/SOURCES/0698-unit-add-jobs-that-were-skipped-because-of-ratelimit.patch @@ -0,0 +1,46 @@ +From dd662fc39a28655b89619a828a15e5e457bf6f4c Mon Sep 17 00:00:00 2001 +From: Michal Sekletar +Date: Thu, 25 Nov 2021 18:28:25 +0100 +Subject: [PATCH] unit: add jobs that were skipped because of ratelimit back to + run_queue + +Assumption in edc027b was that job we first skipped because of active +ratelimit is still in run_queue. Hence we trigger the queue and dispatch +it in the next iteration. Actually we remove jobs from run_queue in +job_run_and_invalidate() before we call unit_start(). Hence if we want +to attempt to run the job again in the future we need to add it back +to run_queue. + +Fixes #21458 + +(cherry picked from commit c29e6a9530316823b0455cd83eb6d0bb8dd664f4) + +Related: #2036608 +--- + src/core/mount.c | 10 ++++++++++ + 1 file changed, 10 insertions(+) + +diff --git a/src/core/mount.c b/src/core/mount.c +index c17154cde1..691b23ca74 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1712,9 +1712,19 @@ static bool mount_is_mounted(Mount *m) { + + static int mount_on_ratelimit_expire(sd_event_source *s, void *userdata) { + Manager *m = userdata; ++ Job *j; ++ Iterator i; + + assert(m); + ++ /* Let's enqueue all start jobs that were previously skipped because of active ratelimit. */ ++ HASHMAP_FOREACH(j, m->jobs, i) { ++ if (j->unit->type != UNIT_MOUNT) ++ continue; ++ ++ job_add_to_run_queue(j); ++ } ++ + /* By entering ratelimited state we made all mount start jobs not runnable, now rate limit is over so + * let's make sure we dispatch them in the next iteration. */ + manager_trigger_run_queue(m); diff --git a/SOURCES/0699-Revert-Revert-sysctl-Enable-ping-8-inside-rootless-P.patch b/SOURCES/0699-Revert-Revert-sysctl-Enable-ping-8-inside-rootless-P.patch new file mode 100644 index 0000000..5f9f27b --- /dev/null +++ b/SOURCES/0699-Revert-Revert-sysctl-Enable-ping-8-inside-rootless-P.patch @@ -0,0 +1,37 @@ +From 54faef034bb2062ed8afa72e2c1be40ef7cc41c5 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Fri, 26 Jul 2019 09:25:09 +0200 +Subject: [PATCH] Revert "Revert "sysctl: Enable ping(8) inside rootless Podman + containers"" + +This reverts commit be74f51605b4c7cb74fec3a50cd13b67598a8ac1. + +Let's add this again. With the new sysctl "-" thing we can make this +work. + +Resolves: #2037807 + +(cherry picked from commit 0338934f4bcda6a96a5342449ae96b003de3378d) +--- + sysctl.d/50-default.conf | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/sysctl.d/50-default.conf b/sysctl.d/50-default.conf +index e0afc9c702..21ae1df13d 100644 +--- a/sysctl.d/50-default.conf ++++ b/sysctl.d/50-default.conf +@@ -33,6 +33,14 @@ net.ipv4.conf.all.accept_source_route = 0 + # Promote secondary addresses when the primary address is removed + net.ipv4.conf.all.promote_secondaries = 1 + ++# ping(8) without CAP_NET_ADMIN and CAP_NET_RAW ++# The upper limit is set to 2^31-1. Values greater than that get rejected by ++# the kernel because of this definition in linux/include/net/ping.h: ++# #define GID_T_MAX (((gid_t)~0U) >> 1) ++# That's not so bad because values between 2^31 and 2^32-1 are reserved on ++# systemd-based systems anyway: https://systemd.io/UIDS-GIDS.html#summary ++net.ipv4.ping_group_range = 0 2147483647 ++ + # Fair Queue CoDel packet scheduler to fight bufferbloat + net.core.default_qdisc = fq_codel + diff --git a/SOURCES/0700-sysctl-prefix-ping-port-range-setting-with-a-dash.patch b/SOURCES/0700-sysctl-prefix-ping-port-range-setting-with-a-dash.patch new file mode 100644 index 0000000..4e2e566 --- /dev/null +++ b/SOURCES/0700-sysctl-prefix-ping-port-range-setting-with-a-dash.patch @@ -0,0 +1,27 @@ +From 41a32aeaf5d33f253f48bfbe8d00de9d160985f7 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Fri, 26 Jul 2019 09:26:07 +0200 +Subject: [PATCH] sysctl: prefix ping port range setting with a dash + +Fixes: #13177 + +Resolves: #2037807 + +(cherry picked from commit 000500c9d6347e0e2cdb92ec48fa10c0bb3ceca8) +--- + sysctl.d/50-default.conf | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/sysctl.d/50-default.conf b/sysctl.d/50-default.conf +index 21ae1df13d..5156d55ca9 100644 +--- a/sysctl.d/50-default.conf ++++ b/sysctl.d/50-default.conf +@@ -39,7 +39,7 @@ net.ipv4.conf.all.promote_secondaries = 1 + # #define GID_T_MAX (((gid_t)~0U) >> 1) + # That's not so bad because values between 2^31 and 2^32-1 are reserved on + # systemd-based systems anyway: https://systemd.io/UIDS-GIDS.html#summary +-net.ipv4.ping_group_range = 0 2147483647 ++-net.ipv4.ping_group_range = 0 2147483647 + + # Fair Queue CoDel packet scheduler to fight bufferbloat + net.core.default_qdisc = fq_codel diff --git a/SOURCES/0701-mount-don-t-propagate-errors-from-mount_setup_unit-f.patch b/SOURCES/0701-mount-don-t-propagate-errors-from-mount_setup_unit-f.patch new file mode 100644 index 0000000..adc7bd1 --- /dev/null +++ b/SOURCES/0701-mount-don-t-propagate-errors-from-mount_setup_unit-f.patch @@ -0,0 +1,56 @@ +From c236734f95550747c4979fe318e3a890adaa0a94 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Wed, 28 Nov 2018 12:41:44 +0100 +Subject: [PATCH] mount: don't propagate errors from mount_setup_unit() further + up + +If we can't process a specific line in /proc/self/mountinfo we should +log about it (which we do), but this should not affect other lines, nor +further processing of mount units. Let's keep these failures local. + +Fixes: #10874 + +Cherry picked from commit ba0d56f55f2073164799be714b5bd1aad94d059a. +Trivial conflict in src/core/mount.c, function mount_load_proc_self_mountinfo, +due to local commit ca634baa10e. Also, due to the same commit, int k +is no longer used and is thus removed. + +Resolves: #2036853 + +Signed-off-by: Kir Kolyshkin +--- + src/core/mount.c | 8 ++------ + 1 file changed, 2 insertions(+), 6 deletions(-) + +diff --git a/src/core/mount.c b/src/core/mount.c +index 691b23ca74..4e0a4f238a 100644 +--- a/src/core/mount.c ++++ b/src/core/mount.c +@@ -1615,12 +1615,10 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) { + if (r < 0) + return log_error_errno(r, "Failed to parse /proc/self/mountinfo: %m"); + +- r = 0; + for (;;) { + struct libmnt_fs *fs; + const char *device, *path, *options, *fstype; + _cleanup_free_ char *d = NULL, *p = NULL; +- int k; + + r = mnt_table_next_fs(table, iter, &fs); + if (r == 1) +@@ -1644,12 +1642,10 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) { + + device_found_node(m, d, DEVICE_FOUND_MOUNT, DEVICE_FOUND_MOUNT); + +- k = mount_setup_unit(m, d, p, options, fstype, set_flags); +- if (r == 0 && k < 0) +- r = k; ++ (void) mount_setup_unit(m, d, p, options, fstype, set_flags); + } + +- return r; ++ return 0; + } + + static void mount_shutdown(Manager *m) { diff --git a/SPECS/systemd.spec b/SPECS/systemd.spec index c57ed16..5a3fb7a 100644 --- a/SPECS/systemd.spec +++ b/SPECS/systemd.spec @@ -13,7 +13,7 @@ Name: systemd Url: http://www.freedesktop.org/wiki/Software/systemd Version: 239 -Release: 54%{?dist} +Release: 55%{?dist} # For a breakdown of the licensing, see README License: LGPLv2+ and MIT and GPLv2+ Summary: System and Service Manager @@ -729,6 +729,28 @@ Patch0676: 0676-tests-add-helper-function-to-autodetect-CI-environme.patch Patch0677: 0677-strv-rework-FOREACH_STRING-macro.patch Patch0678: 0678-test-systemctl-use-const-char-instead-of-char.patch Patch0679: 0679-ci-pass-the-GITHUB_ACTIONS-variable-to-the-CentOS-co.patch +Patch0680: 0680-lgtm-detect-uninitialized-variables-using-the-__clea.patch +Patch0681: 0681-lgtm-replace-the-query-used-for-looking-for-fgets-wi.patch +Patch0682: 0682-lgtm-beef-up-list-of-dangerous-questionnable-API-cal.patch +Patch0683: 0683-lgtm-warn-about-strerror-use.patch +Patch0684: 0684-lgtm-complain-about-accept-people-should-use-accept4.patch +Patch0685: 0685-lgtm-don-t-treat-the-custom-note-as-a-list-of-tags.patch +Patch0686: 0686-lgtm-ignore-certain-cleanup-functions.patch +Patch0687: 0687-lgtm-detect-more-possible-problematic-scenarios.patch +Patch0688: 0688-lgtm-enable-more-and-potentially-useful-queries.patch +Patch0689: 0689-meson-avoid-bogus-meson-warning.patch +Patch0690: 0690-test-make-TEST-47-less-racy.patch +Patch0691: 0691-core-rename-unit_-start_limit-condition-assert-_test.patch +Patch0692: 0692-core-Check-unit-start-rate-limiting-earlier.patch +Patch0693: 0693-sd-event-introduce-callback-invoked-when-event-sourc.patch +Patch0694: 0694-core-rename-generalize-UNIT-u-test_start_limit-hook.patch +Patch0695: 0695-mount-make-mount-units-start-jobs-not-runnable-if-p-.patch +Patch0696: 0696-mount-retrigger-run-queue-after-ratelimit-expired-to.patch +Patch0697: 0697-pid1-add-a-manager_trigger_run_queue-helper.patch +Patch0698: 0698-unit-add-jobs-that-were-skipped-because-of-ratelimit.patch +Patch0699: 0699-Revert-Revert-sysctl-Enable-ping-8-inside-rootless-P.patch +Patch0700: 0700-sysctl-prefix-ping-port-range-setting-with-a-dash.patch +Patch0701: 0701-mount-don-t-propagate-errors-from-mount_setup_unit-f.patch %ifarch %{ix86} x86_64 aarch64 @@ -1356,6 +1378,29 @@ fi %files tests -f .file-list-tests %changelog +* Mon Jan 10 2022 systemd maintenance team - 239-55 +- lgtm: detect uninitialized variables using the __cleanup__ attribute (#2017033) +- lgtm: replace the query used for looking for fgets with a more general query (#2017033) +- lgtm: beef up list of dangerous/questionnable API calls not to make (#2017033) +- lgtm: warn about strerror() use (#2017033) +- lgtm: complain about accept() [people should use accept4() instead, due to O_CLOEXEC] (#2017033) +- lgtm: don't treat the custom note as a list of tags (#2017033) +- lgtm: ignore certain cleanup functions (#2017033) +- lgtm: detect more possible problematic scenarios (#2017033) +- lgtm: enable more (and potentially useful) queries (#2017033) +- test: make TEST-47 less racy (#2017033) +- core: rename unit_{start_limit|condition|assert}_test() to unit_test_xyz() (#2036608) +- core: Check unit start rate limiting earlier (#2036608) +- sd-event: introduce callback invoked when event source ratelimit expires (#2036608) +- core: rename/generalize UNIT(u)->test_start_limit() hook (#2036608) +- mount: make mount units start jobs not runnable if /p/s/mountinfo ratelimit is in effect (#2036608) +- mount: retrigger run queue after ratelimit expired to run delayed mount start jobs (#2036608) +- pid1: add a manager_trigger_run_queue() helper (#2036608) +- unit: add jobs that were skipped because of ratelimit back to run_queue (#2036608) +- Revert "Revert "sysctl: Enable ping(8) inside rootless Podman containers"" (#2037807) +- sysctl: prefix ping port range setting with a dash (#2037807) +- mount: don't propagate errors from mount_setup_unit() further up (#2036853) + * Wed Dec 01 2021 systemd maintenance team - 239-54 - core: consider service with no start command immediately started (#1860899) - man: move description of *Action= modes to FailureAction=/SuccessAction= (#1860899)