From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8EE103590C3 for ; Mon, 2 Feb 2026 11:41:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770032486; cv=none; b=aY69L2MpAQjSBZ92qNkgynX4rwpvJNtxz3fy0h9umTscueOGCGMNhWShAClm2RiyF6e7yb3NCiGRL485vaT7cIyLXZ+a8afAGPsoXgJ2WFeKTOSrFnYOHeCVgd5Yiymj/laFO3VSHo8RAhLBjbkLRADttPUdtvrYJScSDAn3SrA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770032486; c=relaxed/simple; bh=KUzXXlB0yRgiohGOZeLN2o+nzB/PDuppZFlNL3gobv4=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=kXmwsXBHKxcIuKeT2qQlxEsPESJq4dQgon3ymtZkzqrkQxVGSIUGG6W1m0NYhmECQtWgNGMnpwwHsdBXTKcfwdb+kRMKm/vcbkLHRQ3KJF5OkCTolW/2UVbtwaGDOSzTzmSNGlz1oc3WKag3gvFuhWwKyzpQlRAM1NajugksPRQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=KelqkOKq; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=vUu5GSZ3; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="KelqkOKq"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="vUu5GSZ3" From: John Ogness DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1770032484; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=xNng4IljomIONlsVMBBVF1wq1jX8/jcPppmsSY22haw=; b=KelqkOKqUSojHa1veutYYlyX0UFYRyXGeai9LQt1tcWj6gu+aEAx8aqQo7LIq/hI9+YUeC EjT22Y/c3XoeppzIo5XX7J4Wi+8U8jU4erhxJXHmVMazaphIt7RvJXuA2dHusJwHurfvKO ZF6EP1BHkf0BKGhcbE2JMpDQnBl+qxzanSEnX253rR0vgmtaVsWYaXB3hNhcLfsoaIUCno S4OgdZ0+zv1T7ZLAYeQO3LuzsPzd1uuflu0i/LzXwHc4iTODlhfpsuufeah8AHTLGeqIjQ WZuRlCwVBi2b2dcUHYBGJBClCl1rWwahAyBEQpEbF5wcPivzGojaFUkqCr8XIA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1770032484; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=xNng4IljomIONlsVMBBVF1wq1jX8/jcPppmsSY22haw=; b=vUu5GSZ3smZ2kxCHnw29RjqYceBVXamOINh6Uq+ExI6Nuub/4IurcwZ3FPQhfjaA9TGgx5 D5kGwCwkuuC3rkDQ== To: "feng.zhou" , linux-kernel@vger.kernel.org Cc: pmladek@suse.com, senozhatsky@chromium.org, rostedt@goodmis.org, "feng.zhou" Subject: Re: [PATCH] printk: Fix _DESCS_COUNT type for 64-bit systems In-Reply-To: <20260202094140.9518-1-realsummitzhou@gmail.com> References: <20260202094140.9518-1-realsummitzhou@gmail.com> Date: Mon, 02 Feb 2026 12:47:23 +0106 Message-ID: <87o6m7fk4c.fsf@jogness.linutronix.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain Hi Zhou, Thanks for reporting and fixing this! On 2026-02-02, "feng.zhou" wrote: > The _DESCS_COUNT macro currently uses 1U (32-bit unsigned) instead of > 1UL (unsigned long), which breaks the intended overflow testing design > on 64-bit systems. > > Problem Analysis: > ---------------- > The printk_ringbuffer uses a deliberate design choice to initialize > descriptor IDs near the maximum 62-bit value to trigger overflow early > in the system's lifetime. This is documented in printk_ringbuffer.h: > > "initial values are chosen that map to the correct initial array > indexes, but will result in overflows soon." > > The DESC0_ID macro calculates: > DESC0_ID(ct_bits) = DESC_ID(-(_DESCS_COUNT(ct_bits) + 1)) > > On 64-bit systems with typical configuration (descbits=16): > - Current buggy behavior: DESC0_ID = 0xfffeffff > - Expected behavior: DESC0_ID = 0x3ffffffffffeffff > > The buggy version only uses 32 bits, which means: > 1. The initial ID is nowhere near 2^62 > 2. It would take ~140 trillion wraps to trigger 62-bit overflow > 3. The overflow handling code is never tested in practice > > Root Cause: > ---------- > The issue is in this line: > #define _DESCS_COUNT(ct_bits) (1U << (ct_bits)) > > When _DESCS_COUNT(16) is calculated: > 1U << 16 = 0x10000 (32-bit value) > -(0x10000 + 1) = -0x10001 = 0xFFFEFFFF (32-bit two's complement) > > On 64-bit systems, this 32-bit value doesn't get extended to create > the intended 62-bit ID near the maximum value. I was surprised to see that 1U is used here. I did some historical digging and it has existed since the very first private version of this ringbuffer implementation. It was a private email to Petr: Date: 12 Oct 2019 Subject: ringbuffer v1 Message-ID: <87lftqwvhp.fsf@linutronix.de> That was the very first appearance of a descriptor initializer: +#define _DESCS_COUNT(ct_bits) (1U << (ct_bits)) +#define DESCS_COUNT(dr) _DESCS_COUNT((dr)->count_bits) +#define DESC0_ID(ct_bits) DESC_ID(-_DESCS_COUNT(ct_bits)) > Impact: > ------ > While index calculations still work correctly in the short term, this > bug has several implications: > > 1. Violates the design intention documented in the code > 2. Overflow handling code paths remain untested > 3. ABA detection code doesn't get exercised under overflow conditions The ABA detection is only relevant for 32-bit systems. The patch is only fixing an issue on 64-bit systems. > 4. In extreme long-term running scenarios (though unlikely), could > potentially cause issues when ID actually reaches 2^62 It is worth investigating if this is an issue. Although it is quite unlikely. A machine would need to generate 100k printk messages per second for 490k years! > Verification: > ------------ > Tested on ARM64 system with CONFIG_LOG_BUF_SHIFT=20 (descbits=15): > - Before fix: DESC0_ID(16) = 0xfffeffff > - After fix: DESC0_ID(16) = 0x3fffffffffff7fff > > The fix aligns _DESCS_COUNT with _DATA_SIZE, which already correctly > uses 1UL: > #define _DATA_SIZE(sz_bits) (1UL << (sz_bits)) > > Signed-off-by: feng.zhou > --- > kernel/printk/printk_ringbuffer.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h > index 4ef81349d9fb..4f4949700676 100644 > --- a/kernel/printk/printk_ringbuffer.h > +++ b/kernel/printk/printk_ringbuffer.h > @@ -122,7 +122,7 @@ enum desc_state { > }; > > #define _DATA_SIZE(sz_bits) (1UL << (sz_bits)) > -#define _DESCS_COUNT(ct_bits) (1U << (ct_bits)) > +#define _DESCS_COUNT(ct_bits) (1UL << (ct_bits)) > #define DESC_SV_BITS BITS_PER_LONG > #define DESC_FLAGS_SHIFT (DESC_SV_BITS - 2) > #define DESC_FLAGS_MASK (3UL << DESC_FLAGS_SHIFT) The change looks correct. But I would like to take some more time to understand why I never used UL in the first place. Also note (unrelated) that we are using 1U for the buffer of the data ring. Not really an issue, but we should probably switch that to UL as well. John Ogness