From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 88E7743E4A3 for ; Mon, 13 Jul 2026 14:56:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783954599; cv=none; b=AkLd0nu7vr9EakciboWhULKxFo9nRzoPUyuFq1TzuF+DBQ/LP22Fj5yN04wsgToCgjw3ZIbCmelzTzwRRQ10hLM9r0S9c7jcgHzv9bylZyji6OHRJvj1XFp0J4+O2ZdYyujSJcy2pO0pGLRdRdv3CiD+a4DRUJerOhO8ljyYNpM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783954599; c=relaxed/simple; bh=30lKF3fOmvtBdGx4xmCh5Q4gw9770LtPSaKpPDM1Szk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BTqnL5uKfVonpGsFU8nkhDvyKjjbsfUPHP+wt1rrM3UOrG+kchhYGzfW2u8JTQIXBiu5tDpByOIqKd6AYdByofgw91EYtMqfuFzFcV+t5FYSKzBIlVLrXLMgm2Kp0Y3z758W8wzRAvUI8DRuwSanZZVb9ck53g7m9ReFykZi2/U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=gdsn6G3i; arc=none smtp.client-ip=209.85.221.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="gdsn6G3i" Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-475cb71a4ebso3179122f8f.0 for ; Mon, 13 Jul 2026 07:56:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1783954596; x=1784559396; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=pjo1bqqRE1JSlVKTJIbGjEemSKy1fbuPmeSkzy6p9MI=; b=gdsn6G3i3r5jQfhZ82SaNZPrqIuoyGO/aF2fTJEKV/GYk3Qp9Vhfysh3mu2fJtUIDc 0Fyh+2m8qCcknSnvL0RhS/FhQOvQ1fSkTxeZ9qbapZLuu99Hf3OhA6wmu97uZ2w9YuEQ xXogk3jRI6vf+Bs3L/9dyq4MuajPskAVL2KhKWwOGYX2tosS938GDA5IT8/qAMGLG+ez q3BfhlMDgUPUsF6XLmt850zhFSJ2yn00lHXXjZck5I5UQybCyIBrQXCLJLDM0fqRbj1r H6jSHZS6tL5QwJqWg8hYl04fHq8ho+4qwI0BIsqBreRp8hcQQZ/0vuKsECcKU+PKYZq/ ywbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783954596; x=1784559396; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=pjo1bqqRE1JSlVKTJIbGjEemSKy1fbuPmeSkzy6p9MI=; b=GYmgFHFG0g4dujfLwbErMMVeXcxA77KBrzCOlu8GphvA+EJ7RPqy8xmzQ6qaALkG1O 5KUOJKWHcGVD9rEWkQklaZP4fo/u3UxatvwyyjQHKUT4LG4MXkJ9MmNXmE9a9n89HC8E SIzhUZLEEuJzQgCt7v2M4E0hnrvIBd3q9+BA2VcMlocmZSP80Rij24qfC16PZ34w0f0d rOgwEBPxXL6S+YLa2fjpvJWjLdpGVc7QQSghO41LjUvSdbOu/2N2BpXqmBLHwSUXBJpP LWiUDZE1qH+kDc4XDcZEa6kuLOqO0uvrhiyW1C0wu3Ffb+aVqGLsvDVLNYkz+ElFV+kV LygA== X-Forwarded-Encrypted: i=1; AHgh+RpBTG25YAfJaW7jIXrm2tusYpMYRlO8FAtoiFF8PZ2Soc9RyhNEgECjBwJYwK3ll8aXYfkNFBUqC9bm0LY=@vger.kernel.org X-Gm-Message-State: AOJu0YwFx8U7q0CGlpDlgbTDM+7KHXokPnE66kD9dTTRiRYKrrO069p+ jKukFrKay5SRwPwjjEIk0P7SkMIILEuwnTfGk6leX+FhNthpxPgSv1DcgJCIkZ0mY0ylFsXE+4r 8rvY/Kwk= X-Gm-Gg: AfdE7cmXs95o1+2HBS95eKtlmYewU8Yp1/I0uzearaSUSoKRizJF5NQ85Sdww+uSak7 6z+S2YgvGd0b2JWh/D8JkBIGj6B06iRLQDu4hRl3ust8BiyDtw6j7ufgosMg7py4Yrx+SZ8mGzq cCfZm5JDpSx0DL8wpZmPbNhM4mPY1XObN1tsV7X70h0215A0jhHx9AjqtmrwkRlPV5d+lmK4yKG Aj2MMyynOlGF4JBhEOWJhRnFlSJw2A+RaV0gjn9RktJbNnHkLhiQsBLy2nDORUoFWG5G80LESlZ PICPS9J+YZ0p1nMS7eg6ceMoGTD8oy+uKTes3iOnVaJJMXVVTWptN22u7VAb7wEhvd9f9kiLTAj 8lND0HSw5Aktrl7ZJSogzLefYw7WlgKzQ7VFdscjJOFa6J/1mUq0yWSSzdseYE5n/PE08SWclvq 0zRqmz7Ogx/eNjIG0r2nRS4Gf9xQ== X-Received: by 2002:a05:6000:2912:b0:475:3a97:8e2c with SMTP id ffacd0b85a97d-47f2dcb530fmr10678541f8f.16.1783954595654; Mon, 13 Jul 2026 07:56:35 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47a9de1e736sm90714383f8f.7.2026.07.13.07.56.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jul 2026 07:56:35 -0700 (PDT) Date: Mon, 13 Jul 2026 16:56:33 +0200 From: Petr Mladek To: Bradley Morgan Cc: akpm@linux-foundation.org, feng.tang@linux.alibaba.com, linux-kernel@vger.kernel.org, Sashiko Subject: Re: [PATCH v3] panic: fix redirect CPU race in panic_try_force_cpu() Message-ID: References: <20260708163532.18538-1-include@grrlz.net> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260708163532.18538-1-include@grrlz.net> On Wed 2026-07-08 16:35:32, Bradley Morgan wrote: > The cmpxchg() in panic_try_force_cpu() makes sure that only one CPU > tries to redirect panic() to the requested CPU. It is similar to the > cmpxchg() in panic_try_start() which makes sure that only one CPU does > the panic(). In both situations, only the winner of cmpxchg() should > proceed further. Other CPUs should go offline. > > There is a bug in panic_try_force_cpu() because CPUs continue with > vpanic() when the cmpxchg() fails. As a result, they might win in > panic_try_start() instead of the requested CPU. > > The loser cannot just return true, because a CPU that already won the > redirect cmpxchg and then reenters panic(), for example via an NMI > during the string formatting, would fail its own cmpxchg and stop > itself, abandoning the panic. Check old_cpu == this_cpu so a recursive > reentry falls through to panic_try_start() instead. The last paragraph is a bit cryptic. The panic() call in nmi_panic() is neither reentrance nor recursion. panic_try_force_cpu() just can be called twice on the same CPU in this code path. Actually, it is not clear that it can be called twice via nmi_panic() because the patch adding the panic_try_force_cpu() there has not been accepted yet, see https://lore.kernel.org/all/20260708164312.19044-1-include@grrlz.net/ Please, send the related fixes in a single patches so that the ordering is clear. Otherwise, it is a mess. Reported-by: Sashiko > Closes: https://sashiko.dev/#/patchset/20260705164123.18746-1-include@grrlz.net > Closes: https://sashiko.dev/#/patchset/20260707172252.4842-1-include@grrlz.net > > Signed-off-by: Bradley Morgan > --- > kernel/panic.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > Changes since v2: > - Rewrote the commit message to describe the race and the recursive case. > - Used Reported-by and Closes tags. > - Documented the reason return old_cpu != this_cpu is needed. > > diff --git a/kernel/panic.c b/kernel/panic.c > index 03f1eef07b17..ebdc46af6aa9 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -401,11 +401,11 @@ static bool panic_try_force_cpu(const char *fmt, va_list args) > return false; > > /* > - * Only one CPU can do the redirect. Use atomic cmpxchg to ensure > - * we don't race with another CPU also trying to redirect. > + * Only one CPU can do the redirection. Others should go > + * offline. Continue with panic() when ending recursively here. > */ I thought more about this. The recursion is not the right word here. In fact, we should try to avoid panic() recursion because it is likely to end up in an infinite loop. For example, see the commit 1a2383e8b84c0451fd9 ("panic: unset panic_on_warn inside panic()"). IMHO, the panic() recursion might happen. But I think that our primary concern is calling panic_try_force_cpu() twice on the same CPU via in nmi_panic(), see https://lore.kernel.org/all/20260708164312.19044-1-include@grrlz.net/ A better comment might be: /* * Only one CPU can do the redirection. Others should go offline [*]. * Continue with panic() when we already tried the redirection * from this CPU before, for example, in nmi_panic(). * * [*] Note that we are here only when this_cpu != panic_redirect_cpu. */ But this would make sense only when nmi_panic() is updated first. It might make sense to merge the two patches. Otherewise, there might be an Chicken & Egg problem. Note that this change is needed also because of the potential panic() recursion. But I do not want to express it that much. It might make false feeling that panic() recursion is fine. > if (!atomic_try_cmpxchg(&panic_redirect_cpu, &old_cpu, this_cpu)) > - return false; > + return old_cpu != this_cpu; > > /* > * Use dynamically allocated buffer if available, otherwise Best Regards, Petr