From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f169.google.com (mail-pl1-f169.google.com [209.85.214.169]) (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 75F2E392825 for ; Tue, 4 Aug 2026 12:10:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785845450; cv=none; b=NopCmBBXVsU40tdz88gk2MicVekybiHFYG39ucHZmFkgubvS6A56ltL1zYtYdpjTeQpFZWImAv5+TKQgQgXMCvLXWCYXvXD25QQafVO6cnE9kn98hL3lZ3PGvV96e2Nro4VNgOq2cPbcSmnslgiQxVN/18A1nW21uBStlWNTCzU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785845450; c=relaxed/simple; bh=UVcZ8TZVOO8MxzMJhucuxEeLaGW/RR2rfo/mzbWKJAk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ICJcvoJgWVQ7JPNc62YOw5lU8PpxTnCWc66m3mpEHDuTUTYgvCkkACZ78OnBSN2kzhWaIdPA5omf5Ou0IzB6HWSuXdddRW34dCaJdxtPaTIdM1bXR5Ct+Ql/VlIctOzecyWvkuT4k1b+zx9OeCS4iFilOEXoGrcQZMueoPinSM0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=UXWqcbrl; arc=none smtp.client-ip=209.85.214.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="UXWqcbrl" Received: by mail-pl1-f169.google.com with SMTP id d9443c01a7336-2cf50c6f235so50689125ad.0 for ; Tue, 04 Aug 2026 05:10:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1785845449; x=1786450249; 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=5YzKl/HTx9hxe4H/h1xhyOcFbjFzNfWrOyFbMdFBHhY=; b=UXWqcbrljP8X4co7ck67DCNleqVOpVBpsxEdXHEOLWSYA40ARw/QyeV2EbgvCJgU9K KD63Dy2zJgnJGVVmM4oTMOKluoKOnsSzLHOQwZauq9nki4vVP5imgrtNWlaE5qm5am8N Q3vOKYMZy0MQ4RqVA2VcKbnLLnItHW666CKjmZGldAY70LG6uveNBTtb4RXNr/IPzIV6 G0cMn6eyoJPNuqV4LGKcD6CKB0PIjr8/faT5D0TFPQgZpeFNoHnMgJCaeKpyJGD8MPF/ iQMgF57GQ9BhvUNTEhJtIjRT4RZhks3N+OeSGbpFT/YFWHgDggy2t5ETBx/wSqiYJLmx JNJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785845449; x=1786450249; 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=5YzKl/HTx9hxe4H/h1xhyOcFbjFzNfWrOyFbMdFBHhY=; b=tB9LWspINfYlw8BIRV4jETTCQWzOYgZMrkzifSjf56IOqLh5ft6YhKb4UJjVCFnahL 7FsMesl8vwpZDSJI3Sle3I6avw6VyXT3zdXmQ52+fSaFnDfqNdvoKkXLnV9hQ5U+EAma HHy68zq5ceBy2cqxmj4/LVNXPsj47giubtSsvgZI+df9nzeXteUWByEyUgcwv7BxkUjw 2Omxern782lr5XliZGIbhr8TsCmRxfSMfCgJUjjv0rQO/Beunr1QRji55h04VULZFukv RSytBvDclGQopkALT4HhKPOonK1o6OkplF8d7SCq0cioxVYASiaftC1uKlQkw26LXuK7 68Ww== X-Forwarded-Encrypted: i=1; AHgh+RoH0Z3eqU92PPfxHeMDKK4Pnr3cy4CPYFr1Tg0+OSVPDm+0rI7zT1XlHxIxSPBX0uPDmR4ulgsW+235A5c=@vger.kernel.org X-Gm-Message-State: AOJu0Yy/KcN/ciY2afgtYf2dWv16ndDPZ9B+83lZEjn3mk/oFVx4Q49w P5ny7QYnSUghMw9bzYDVF9zDJ/0wwnlmlRwqT4gJRcYpWToutvmIIKkedBliJUf086U= X-Gm-Gg: AR+sD11v9tI6ANI45SSFvhyvNoAZm/5ydnA3/yFS4f1VcZrR10s95TGdL2rsb4+/bf2 aPII/gv5WitSzRP3HQ3YqhiQGVXFYu2wIlOxNgKzkWRXkPB0jgwXm1q2T/qCjueNWmwviE0q+Gn Gvm/lhnocX5kizjkDcLBWABXq7cgkBocJ4O25wNOX1/K5sxVamI3+f2n1z1xTEz4mN5saGh3yLg IQgGaG3wLT+6829aAMVDM1mXJblCblQEO5/Q67XdMic6PSqFSzDCj9BqN9MRM5F7w0RSxuHara6 N6ZA9+IW//OJF9f54y9+dDBoyxdHu5PTtwVWBzTBC3XePNwnsXaFKR7Z2eDDAkx32ekjwQYV8xt Lv5PYmaII6diK+TvGsS2BxOuZrZdDQLAeZX5wkcKWeTsorS/ASSXPjW0D6P+K1ZowZ8AEF/TSpd na0nwjWgRDID+XqPDwgo4oPy2WUcrM3RJ3HK714YrIyeqhy800R8tJjw== X-Received: by 2002:a17:903:19e6:b0:2c7:f2c6:89e0 with SMTP id d9443c01a7336-2d0521eabe6mr152709145ad.4.1785845448701; Tue, 04 Aug 2026 05:10:48 -0700 (PDT) Received: from localhost ([172.216.252.8]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2d0a9f8fb4bsm6289705ad.5.2026.08.04.05.10.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Aug 2026 05:10:47 -0700 (PDT) Date: Fri, 31 Jul 2026 14:55:24 +0300 From: Dan Carpenter To: Mohit Mishra Cc: Greg Kroah-Hartman , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: rtl8723bs: fix underflow logic in swing index calculations Message-ID: References: <20260729172707.13333-1-mishraloopmohit@gmail.com> 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: <20260729172707.13333-1-mishraloopmohit@gmail.com> On Wed, Jul 29, 2026 at 10:57:07PM +0530, Mohit Mishra wrote: > In ODM_TxPwrTrackSetPwr_8723B(), the baseband swing index variables > Final_OFDM_Swing_Index and Final_CCK_Swing_Index are declared as u8. > However, their calculation adds Absolute_OFDMSwingIdx, which is a signed > 8-bit integer (s8) and can be negative: > > Final_OFDM_Swing_Index = pDM_Odm->DefaultOfdmIndex + > pDM_Odm->Absolute_OFDMSwingIdx[RFPath]; > > If the resulting sum is negative, it underflows under u8 rules (e.g. -5 > becomes 251). This causes the lower-limit checks (e.g. <= 0) to fail, > and in MIX_MODE causes the logic to execute the "BBSwing higher than limit" > branch instead of capping to 0. > > Additionally, in BBSWING mode, the check for CCK underflow mistakenly > examines the static struct member pDM_Odm->BbSwingIdxCck instead of the > newly calculated Final_CCK_Swing_Index: > > else if (pDM_Odm->BbSwingIdxCck <= 0) > This is a separate thing and needs to be in a separate patch with a Fixes tag. > Fix this by changing both swing index variable types to int to enable > signed math and correct branch selection (aligning with the TODO item to > convert remaining unusual variable types). Update the CCK check in > BBSWING mode to examine Final_CCK_Swing_Index. > > Note: The fix is scoped to the calculation and branching logic. When > passed downstream to setIqkMatrix_8723B() and setCCKFilterCoefficient(), > the values are already clamped within [0, 42], fitting safely in u8. > > Compile-tested only; no hardware available for testing. > > Signed-off-by: Mohit Mishra > --- The type issue is fundamentally a static checker type bugfix that unsigned values can't be less than zero. Linus's take on that is that this code: if (x < 0 || x > limit) { is perfectly fine and readable as a clamp even when x is unsigned. In this case the commit message has a lot of extra discussion about how the math could lead to a negative value because we entered negative data or we had an integer overflow etc. The result is that we clamped it to zero instead of to the upper bound. There is no evidence that any of this is possible in real life. And also if it were who cares? Zero is a valid value. If you use a complicated integer overflow method to get zero instead of just doing it the normal way, the result is the same... The code is pure garbage, of course. I also have written that choosing u8 for this type of variable is dumb: https://staticthinking.wordpress.com/2022/06/01/unsigned-int-i-is-stupid/ I don't object to fixing this code as part of a cleanup but the commit message needs to be more clear that were cleaning it up because the code is garbage and not because of some kind of complicated safety issue. regards, dan carpenter