linux-kernel - Re: [GIT PULL] s390 fixes for 6.6-rc7

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <your-ad-here.call-01697980674-ext-9589@work.hours>
Date:   Sun, 22 Oct 2023 15:17:54 +0200
From:   Vasily Gorbik <gor@...ux.ibm.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andy Shevchenko <andriy.shevchenko@...el.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org
Subject: Re: [GIT PULL] s390 fixes for 6.6-rc7

On Sat, Oct 21, 2023 at 11:08:31AM -0700, Linus Torvalds wrote:
> in case you or somebody has a better idea for BITS_TO_LONG handling
> than just "you need to check for zero before and after".
> 
> On Sat, 21 Oct 2023 at 10:56, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > If you *do* want to add proper overflow handling, you'd need to either
> > fix BITS_TO_LONGS() some way (which is actually non-trivial since it
> > needs to be able to stay a constant and only use the argument once),
> > or you do something like
> >
> >         if (!bits)
> >                 return ZERO_SIZE_PTR;
> >         longs = BITS_TO_LONG(bits);
> >         if (!longs)
> >                 return NULL;
> >         return vzalloc(longs * sizeof(long));

This might work.

BITS_TO_<TYPE>(bits) utilizes __KERNEL_DIV_ROUND_UP, which may potentially
result in an overflow condition when

	bits > ULONG_MAX - sizeof(<TYPE>) * 8 + 1.

To resolve this issue, avoid using the overflow-prone
__KERNEL_DIV_ROUND_UP. To meet the requirements of BITS_TO<TYPE>(bits)
for remaining constant and preventing side effects from multiple
argument uses, employ __is_constexpr to differentiate between constant
and non-constant cases, employing a helper function in the latter.

In the constant case, this ensures compatibility with constructs like
DECLARE_BITMAP. While in the non-constant case, the __bits_to_elem_count
function could be optimized for potentially improved code generation
by compilers, though this might come at the expense of readability and
visual consistency between the constant and non-constant cases.
I could further investigate if this approach, in general, appears acceptable.
---
 include/linux/bitops.h | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 2ba557e067fe..72be25d4b95d 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -15,11 +15,21 @@
 #  define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n)))
 #endif
 
+static inline unsigned long __bits_to_elem_count(size_t nr, size_t sz)
+{
+	return nr / sz + (nr % sz ? 1 : 0);
+}
+
+#define BITS_TO_ELEM_COUNT(nr, sz)					\
+	__builtin_choose_expr(__is_constexpr(nr),			\
+			     (nr) / sz + ((nr) % sz ? 1 : 0),		\
+			     __bits_to_elem_count((nr), sz))
+
 #define BITS_PER_TYPE(type)	(sizeof(type) * BITS_PER_BYTE)
-#define BITS_TO_LONGS(nr)	__KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
-#define BITS_TO_U64(nr)		__KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u64))
-#define BITS_TO_U32(nr)		__KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u32))
-#define BITS_TO_BYTES(nr)	__KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(char))
+#define BITS_TO_LONGS(nr)	BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(long))
+#define BITS_TO_U64(nr)		BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(u64))
+#define BITS_TO_U32(nr)		BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(u32))
+#define BITS_TO_BYTES(nr)	BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(char))
 
 extern unsigned int __sw_hweight8(unsigned int w);
 extern unsigned int __sw_hweight16(unsigned int w);
-- 
2.39.2