linux-kernel - [PATCH] tick: Invalid ratelimit check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Aug 2022 13:26:42 -0700
From:   Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
To:     fweisbec@...il.com, tglx@...utronix.de, mingo@...nel.org
Cc:     linux-kernel@...r.kernel.org,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        stable@...r.kernel.org
Subject: [PATCH] tick: Invalid ratelimit check

In the function report_idle_softirq(), ratelimit is a static variable
and checked for < 10 to return false. Since this variable is not
assigned to any other value before the check, this condition will
always be true.

report_idle_softirq() introduced with the
commit 0345691b24c0 ("tick/rcu: Stop allowing RCU_SOFTIRQ in idle")
checks for pending soft irq during stopping of tick and returns true if
tick can't be stopped.

The purpose of ratelimit is to limit printing of warning messages, so
move the check for printing warning message only. Also don't return
false as tick can't be stopped here for pending soft irq.

Fixes: 0345691b24c0 ("tick/rcu: Stop allowing RCU_SOFTIRQ in idle")
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc: stable@...r.kernel.org # 5.18+
---

One more change from the prior version of kernel:
In prior version !local_bh_blocked() check was only used for printing
warning message. The output was not used to decide whether to allow
tick-stop or not.  But with change introduced with 5.18, the output
will also decide whether to stop tick or not even if
local_softirq_pending() returns true.
Not sure if this is intentional change or not.

 kernel/time/tick-sched.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index b0e3c9205946..4db634525bf4 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1025,16 +1025,15 @@ static bool report_idle_softirq(void)
 			return false;
 	}

-	if (ratelimit < 10)
-		return false;
-
 	/* On RT, softirqs handling may be waiting on some lock */
 	if (!local_bh_blocked())
 		return false;

-	pr_warn("NOHZ tick-stop error: local softirq work is pending, handler #%02x!!!\n",
-		pending);
-	ratelimit++;
+	if (ratelimit < 10) {
+		pr_warn("NOHZ tick-stop error: local softirq work is pending, handler #%02x!!!\n",
+			pending);
+		ratelimit++;
+	}

 	return true;
 }
-- 
2.35.3