linux-kernel - Re: [RFC PATCH] math.h: Account for 64-bit division on i386

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2bf6d797-d974-49bb-a234-a20378f5dda5@roeck-us.net>
Date: Sun, 20 Apr 2025 10:42:18 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: cem@...nel.org
Cc: linux-kernel@...r.kernel.org, Hans.Holmberg@....com,
	oe-kbuild-all@...ts.linux.dev, hch@....de, lukas@...ner.de,
	angelogioacchino.delregno@...labora.com,
	Jonathan.Cameron@...wei.com
Subject: Re: [RFC PATCH] math.h: Account for 64-bit division on i386

On Sun, Apr 20, 2025 at 08:40:27AM -0700, Guenter Roeck wrote:
> On Sat, Apr 19, 2025 at 01:51:46PM +0200, cem@...nel.org wrote:
> > From: Carlos Maiolino <cem@...nel.org>
> > 
> > Building linux on i386 might fail if a 64bit type is passed to
> 
> i386 actually builds. Its compiler is probably able to convert
> the offending mult_frac() without helpers since the divisor is
> a constant. I see the problem with openrisc and parisc, with
> gcc 13.3.0.
> 
> > mult_fract(). To prevent the failure, use do_div() for the division
> > calculation instead of hardcoding a / b.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@...hat.com>
> > Reported-by: kernel test robot <lkp@...el.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/
> > ---
> > 
> > I'm sending it as a RFC because I didn't to extensive testing on this
> > patch, also I'm not sure if mult_frac() was intended to work on 32-bit
> > only types. If that's the case, perhaps, a new mult_frac64() might be a
> > better idea?!
> > 
> >  include/linux/math.h | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/linux/math.h b/include/linux/math.h
> > index 0198c92cbe3e..05ea853b75b4 100644
> > --- a/include/linux/math.h
> > +++ b/include/linux/math.h
> > @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32)
> >  #undef __STRUCT_FRACT
> >  
> >  /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */
> > -#define mult_frac(x, n, d)	\
> > -({				\
> > -	typeof(x) x_ = (x);	\
> > -	typeof(n) n_ = (n);	\
> > -	typeof(d) d_ = (d);	\
> > -				\
> > -	typeof(x_) q = x_ / d_;	\
> > -	typeof(x_) r = x_ % d_;	\
> > -	q * n_ + r * n_ / d_;	\
> > +#define mult_frac(x, n, d)		\
> > +({					\
> > +	typeof(x) x_ = (x);		\
> > +	typeof(n) n_ = (n);		\
> > +	typeof(d) d_ = (d);		\
> > +					\
> > +	typeof(x_) r = do_div(x_, d_);	\
> > +	r *= n_;			\
> > +	do_div(r, d_);			\
> > +	x_ * n_ + r;			\
> >  })
> >  
> 
> Unfortunately that doesn't work. I get build errors on parisc.
> 

Turns out the first parameter of do_div needs to be u64, not s64,
at least on parisc.

Guenter

> In file included from ./arch/parisc/include/generated/asm/div64.h:1,
>                  from ./include/linux/math.h:6,
>                  from ./include/linux/kernel.h:27,
>                  from ./arch/parisc/include/asm/bug.h:5,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/mmdebug.h:5,
>                  from ./include/linux/mm.h:6,
>                  from mm/page_alloc.c:19:
> mm/page_alloc.c: In function 'boost_watermark':
> ./include/asm-generic/div64.h:183:35: error: comparison of distinct pointer types lacks a cast [-Werror]
>   183 |         (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
>       |                                   ^~
> ./include/linux/math.h:142:24: note: in expansion of macro 'do_div'
>   142 |         typeof(x_) r = do_div(x_, d_);  \
>       |                        ^~~~~~
> mm/page_alloc.c:2010:21: note: in expansion of macro 'mult_frac'
>  2010 |         max_boost = mult_frac(zone->_watermark[WMARK_HIGH],
> 
> That is just one example. It seems to affect all uses of
> mult_frac().
> 
> Guenter