Received-SPF: pass (google.com: domain of linux-kernel+bounces-60135-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
To: linux-kernel@vger.kernel.org
Cc: David.Laight@ACULAB.COM,
	pmalani@chromium.org,
	andriy.shevchenko@linux.intel.com,
	keescook@chromium.org,
	torvalds@linux-foundation.org,
	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Herve Codina <herve.codina@bootlin.com>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>
Subject: [PATCH] minmax: Add notes to min_t and max_t
Date: Fri,  9 Feb 2024 15:07:02 -0800
Message-ID: <20240209150657.1.I45addf7579e1233fa97c05ba72120cd1c57b4310@changeid>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Both min_t and max_t are problematic as they can hide issues when
comparing differently sized types (and especially differently signed
types). Update the comments to nudge users to other options until
there is a better fix for these macros.

Link: https://lore.kernel.org/all/01e3e09005e9434b8f558a893a47c053@AcuMS.aculab.com/
Link: https://lore.kernel.org/all/CAHk-=whwEAc22wm8h9FESPB5X+P4bLDgv0erBQMa1buTNQW7tA@mail.gmail.com/

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
Andy Shevchenko made me aware of this particular footgun in
https://lore.kernel.org/linux-usb/ZcZ_he1jYx8w57mK@smile.fi.intel.com/.

While David + others work on the full fix, I'm hoping to apply a
bandaid in the form of comments so the problem doesn't get worse by devs
(**cough** me **cough**) inadvertently doing the wrong thing.


 include/linux/minmax.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2ec559284a9f..96646f840a1f 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -154,6 +154,18 @@
 
 /**
  * min_t - return minimum of two values, using the specified type
+ *
+ * Note: Downcasting types in this macro can cause incorrect results. Prefer to
+ * use min() which does typechecking.
+ *
+ * Prefer to use clamp if you are trying to compare to size_t.
+ *
+ * Don't:
+ *   min_t(size_t, buf_size, sizeof(foobar))
+ *
+ * Do:
+ *  clamp(buf_size, 0, sizeof(foobar))
+ *
  * @type: data type to use
  * @x: first value
  * @y: second value
@@ -162,6 +174,10 @@
 
 /**
  * max_t - return maximum of two values, using the specified type
+ *
+ * Note: Downcasting types in this macro can cause incorrect results. Prefer to
+ * use max() which does typechecking.
+ *
  * @type: data type to use
  * @x: first value
  * @y: second value
-- 
2.43.0.687.g38aa6559b0-goog