do_div() silently truncates "base" to 32bit

do_div() silently truncates "base" to 32bit

[Anatol Pomozov]

Hi,


I was debugging weird "zero divide" problem in CFQ code below


static u64 cfqg_prfill_avg_queue_size(struct seq_file *sf,
      struct blkg_policy_data *pd, int off)
{
    struct cfq_group *cfqg = pd_to_cfqg(pd);
    u64 samples = blkg_stat_read(&cfqg->stats.avg_queue_size_samples);
    u64 v = 0;

    if (samples) {
        v = blkg_stat_read(&cfqg->stats.avg_queue_size_sum);
        do_div(v, samples);
    }
    __blkg_prfill_u64(sf, pd, v);
    return 0;
}


do_div() crashes says "zero divide". It is weird because just a few
lines above we check divider for zero.


The problem comes from include/asm-generic/div64.h file that
implements do_div() as macros:

# define do_div(n,base) ({ \
    uint32_t __base = (base); \
    uint32_t __rem; \
    __rem = ((uint64_t)(n)) % __base; \
    (n) = ((uint64_t)(n)) / __base; \
    __rem; \
 })


Do you see the problem?

The problem here is that "base" argument is truncated to 32bit, but in
the function above "sample" is 64bit variable. If sample's 32 low bits
are zero - we have a crash. in fact we have incorrect behavior any
time when high 32bits are non-zero.


My question is why the base is 32bit? Why not to use 64bit arguments?

Ideally if this macros is converted to a function so compiler will
warn us about unexpected truncation like this. But in this case it
will be hard to do as "n" parameter both input and output.

Re: do_div() silently truncates "base" to 32bit

[Randy Dunlap]

On 08/30/13 10:21, Anatol Pomozov wrote:
> Hi,
> 
> 
> I was debugging weird "zero divide" problem in CFQ code below
> 
> 
> static u64 cfqg_prfill_avg_queue_size(struct seq_file *sf,
>       struct blkg_policy_data *pd, int off)
> {
>     struct cfq_group *cfqg = pd_to_cfqg(pd);
>     u64 samples = blkg_stat_read(&cfqg->stats.avg_queue_size_samples);
>     u64 v = 0;
> 
>     if (samples) {
>         v = blkg_stat_read(&cfqg->stats.avg_queue_size_sum);
>         do_div(v, samples);
>     }
>     __blkg_prfill_u64(sf, pd, v);
>     return 0;
> }
> 
> 
> do_div() crashes says "zero divide". It is weird because just a few
> lines above we check divider for zero.
> 
> 
> The problem comes from include/asm-generic/div64.h file that
> implements do_div() as macros:
> 
> # define do_div(n,base) ({ \
>     uint32_t __base = (base); \
>     uint32_t __rem; \
>     __rem = ((uint64_t)(n)) % __base; \
>     (n) = ((uint64_t)(n)) / __base; \
>     __rem; \
>  })
> 
> 
> Do you see the problem?
> 
> The problem here is that "base" argument is truncated to 32bit, but in
> the function above "sample" is 64bit variable. If sample's 32 low bits
> are zero - we have a crash. in fact we have incorrect behavior any
> time when high 32bits are non-zero.
> 
> 
> My question is why the base is 32bit? Why not to use 64bit arguments?

Maybe performance related?

If you want 64-bit values, don't use do_div() from asm-generic/div64.h.

Instead look at linux/math64.h and use div_u64_rem() et al
or the recently posted div64_u64_rem().
[posted by Mike Snitzer on Aug. 21 2013]

I.e., use exactly the function(s) that you need to use.

Does that fix the problem?


> Ideally if this macros is converted to a function so compiler will
> warn us about unexpected truncation like this. But in this case it
> will be hard to do as "n" parameter both input and output.
> --


-- 
~Randy

Re: do_div() silently truncates "base" to 32bit

[Anatol Pomozov]

Hi

On Fri, Aug 30, 2013 at 2:23 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 08/30/13 10:21, Anatol Pomozov wrote:
>> Hi,
>>
>>
>> I was debugging weird "zero divide" problem in CFQ code below
>>
>>
>> static u64 cfqg_prfill_avg_queue_size(struct seq_file *sf,
>>       struct blkg_policy_data *pd, int off)
>> {
>>     struct cfq_group *cfqg = pd_to_cfqg(pd);
>>     u64 samples = blkg_stat_read(&cfqg->stats.avg_queue_size_samples);
>>     u64 v = 0;
>>
>>     if (samples) {
>>         v = blkg_stat_read(&cfqg->stats.avg_queue_size_sum);
>>         do_div(v, samples);
>>     }
>>     __blkg_prfill_u64(sf, pd, v);
>>     return 0;
>> }
>>
>>
>> do_div() crashes says "zero divide". It is weird because just a few
>> lines above we check divider for zero.
>>
>>
>> The problem comes from include/asm-generic/div64.h file that
>> implements do_div() as macros:
>>
>> # define do_div(n,base) ({ \
>>     uint32_t __base = (base); \
>>     uint32_t __rem; \
>>     __rem = ((uint64_t)(n)) % __base; \
>>     (n) = ((uint64_t)(n)) / __base; \
>>     __rem; \
>>  })
>>
>>
>> Do you see the problem?
>>
>> The problem here is that "base" argument is truncated to 32bit, but in
>> the function above "sample" is 64bit variable. If sample's 32 low bits
>> are zero - we have a crash. in fact we have incorrect behavior any
>> time when high 32bits are non-zero.
>>
>>
>> My question is why the base is 32bit? Why not to use 64bit arguments?
>
> Maybe performance related?
>
> If you want 64-bit values, don't use do_div() from asm-generic/div64.h.
>
> Instead look at linux/math64.h and use div_u64_rem() et al
> or the recently posted div64_u64_rem().
> [posted by Mike Snitzer on Aug. 21 2013]
>
> I.e., use exactly the function(s) that you need to use.
>
> Does that fix the problem?

It definitely fixes the crash. I've already sent a patch to CFQ
maillist http://news.gmane.org/gmane.linux.kernel.cgroups

But another question still remains: why compiler does not warn that
size truncation happens? How to prevent bugs like CFQ one in the
future? Should we add a compile-time assert to do_div() to prevent
passing 64 numbers in "base" macro parameter?

Re: do_div() silently truncates "base" to 32bit

[Randy Dunlap]

On 08/30/13 15:14, Anatol Pomozov wrote:
> Hi
> 
> On Fri, Aug 30, 2013 at 2:23 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> On 08/30/13 10:21, Anatol Pomozov wrote:
>>> Hi,
>>>
>>>
>>> I was debugging weird "zero divide" problem in CFQ code below
>>>
>>>
>>> static u64 cfqg_prfill_avg_queue_size(struct seq_file *sf,
>>>       struct blkg_policy_data *pd, int off)
>>> {
>>>     struct cfq_group *cfqg = pd_to_cfqg(pd);
>>>     u64 samples = blkg_stat_read(&cfqg->stats.avg_queue_size_samples);
>>>     u64 v = 0;
>>>
>>>     if (samples) {
>>>         v = blkg_stat_read(&cfqg->stats.avg_queue_size_sum);
>>>         do_div(v, samples);
>>>     }
>>>     __blkg_prfill_u64(sf, pd, v);
>>>     return 0;
>>> }
>>>
>>>
>>> do_div() crashes says "zero divide". It is weird because just a few
>>> lines above we check divider for zero.
>>>
>>>
>>> The problem comes from include/asm-generic/div64.h file that
>>> implements do_div() as macros:
>>>
>>> # define do_div(n,base) ({ \
>>>     uint32_t __base = (base); \
>>>     uint32_t __rem; \
>>>     __rem = ((uint64_t)(n)) % __base; \
>>>     (n) = ((uint64_t)(n)) / __base; \
>>>     __rem; \
>>>  })
>>>
>>>
>>> Do you see the problem?
>>>
>>> The problem here is that "base" argument is truncated to 32bit, but in
>>> the function above "sample" is 64bit variable. If sample's 32 low bits
>>> are zero - we have a crash. in fact we have incorrect behavior any
>>> time when high 32bits are non-zero.
>>>
>>>
>>> My question is why the base is 32bit? Why not to use 64bit arguments?
>>
>> Maybe performance related?
>>
>> If you want 64-bit values, don't use do_div() from asm-generic/div64.h.
>>
>> Instead look at linux/math64.h and use div_u64_rem() et al
>> or the recently posted div64_u64_rem().
>> [posted by Mike Snitzer on Aug. 21 2013]
>>
>> I.e., use exactly the function(s) that you need to use.
>>
>> Does that fix the problem?
> 
> It definitely fixes the crash. I've already sent a patch to CFQ
> maillist http://news.gmane.org/gmane.linux.kernel.cgroups
> 
> But another question still remains: why compiler does not warn that
> size truncation happens? How to prevent bugs like CFQ one in the
> future? Should we add a compile-time assert to do_div() to prevent
> passing 64 numbers in "base" macro parameter?


That sounds like a fine idea to me.

-- 
~Randy

Re: do_div() silently truncates "base" to 32bit

[Joe Perches]

On Fri, 2013-08-30 at 15:48 -0700, Randy Dunlap wrote:
> On 08/30/13 15:14, Anatol Pomozov wrote:
> > But another question still remains: why compiler does not warn that
> > size truncation happens? How to prevent bugs like CFQ one in the
> > future? Should we add a compile-time assert to do_div() to prevent
> > passing 64 numbers in "base" macro parameter?
> That sounds like a fine idea to me.

Geert thought so too and submitted a patch

http://www.spinics.net/lists/linux-btrfs/msg26788.html

Re: do_div() silently truncates "base" to 32bit

[Anatol Pomozov]

Hi, Joe

On Fri, Aug 30, 2013 at 4:28 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2013-08-30 at 15:48 -0700, Randy Dunlap wrote:
>> On 08/30/13 15:14, Anatol Pomozov wrote:
>> > But another question still remains: why compiler does not warn that
>> > size truncation happens? How to prevent bugs like CFQ one in the
>> > future? Should we add a compile-time assert to do_div() to prevent
>> > passing 64 numbers in "base" macro parameter?
>> That sounds like a fine idea to me.
>
> Geert thought so too and submitted a patch
>
> http://www.spinics.net/lists/linux-btrfs/msg26788.html

Thanks it works!

block/cfq-iosched.c: In function 'cfqg_prfill_avg_queue_size':
block/cfq-iosched.c:4423:3: error: size of unnamed array is negative
make[2]: *** [block/gfq-iosched.s] Error 1
make[1]: *** [block/gfq-iosched.s] Error 2



I see a number of other truncation errors. We need to fix them as well.

Re: do_div() silently truncates "base" to 32bit

[Anatol Pomozov]

Hi

On Fri, Aug 30, 2013 at 5:50 PM, Anatol Pomozov
<anatol.pomozov@gmail.com> wrote:
> Hi, Joe
>
> On Fri, Aug 30, 2013 at 4:28 PM, Joe Perches <joe@perches.com> wrote:
>> On Fri, 2013-08-30 at 15:48 -0700, Randy Dunlap wrote:
>>> On 08/30/13 15:14, Anatol Pomozov wrote:
>>> > But another question still remains: why compiler does not warn that
>>> > size truncation happens? How to prevent bugs like CFQ one in the
>>> > future? Should we add a compile-time assert to do_div() to prevent
>>> > passing 64 numbers in "base" macro parameter?
>>> That sounds like a fine idea to me.
>>
>> Geert thought so too and submitted a patch
>>
>> http://www.spinics.net/lists/linux-btrfs/msg26788.html
>
> Thanks it works!
>
> block/cfq-iosched.c: In function 'cfqg_prfill_avg_queue_size':
> block/cfq-iosched.c:4423:3: error: size of unnamed array is negative
> make[2]: *** [block/gfq-iosched.s] Error 1
> make[1]: *** [block/gfq-iosched.s] Error 2
> I see a number of other truncation errors. We need to fix them as well.

I looked at the places where do_div() passes incorrect parameters and
it is quite a lot of them. It would be better to warn users until all
the places are fixed. Is there any BUILD_WARN_ON() macro?

Re: do_div() silently truncates "base" to 32bit

[Randy Dunlap]

On 09/04/13 08:32, Anatol Pomozov wrote:
> Hi
> 
> On Fri, Aug 30, 2013 at 5:50 PM, Anatol Pomozov
> <anatol.pomozov@gmail.com> wrote:
>> Hi, Joe
>>
>> On Fri, Aug 30, 2013 at 4:28 PM, Joe Perches <joe@perches.com> wrote:
>>> On Fri, 2013-08-30 at 15:48 -0700, Randy Dunlap wrote:
>>>> On 08/30/13 15:14, Anatol Pomozov wrote:
>>>>> But another question still remains: why compiler does not warn that
>>>>> size truncation happens? How to prevent bugs like CFQ one in the
>>>>> future? Should we add a compile-time assert to do_div() to prevent
>>>>> passing 64 numbers in "base" macro parameter?
>>>> That sounds like a fine idea to me.
>>>
>>> Geert thought so too and submitted a patch
>>>
>>> http://www.spinics.net/lists/linux-btrfs/msg26788.html
>>
>> Thanks it works!
>>
>> block/cfq-iosched.c: In function 'cfqg_prfill_avg_queue_size':
>> block/cfq-iosched.c:4423:3: error: size of unnamed array is negative
>> make[2]: *** [block/gfq-iosched.s] Error 1
>> make[1]: *** [block/gfq-iosched.s] Error 2
>> I see a number of other truncation errors. We need to fix them as well.
> 
> I looked at the places where do_div() passes incorrect parameters and
> it is quite a lot of them. It would be better to warn users until all
> the places are fixed. Is there any BUILD_WARN_ON() macro?
> 


Not that I know of or can find.

-- 
~Randy

[PATCH] core: Catch overflows in do_div() function

[Anatol Pomozov]

If second parameter passed to this function was 64 then it silently
truncates to 32 bits. Catch such situation.

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 include/asm-generic/div64.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
index 8f4e319..84339a0 100644
--- a/include/asm-generic/div64.h
+++ b/include/asm-generic/div64.h
@@ -17,6 +17,7 @@
  *       beware of side effects!
  */
 
+#include <linux/bug.h>
 #include <linux/types.h>
 #include <linux/compiler.h>
 
@@ -25,6 +26,7 @@
 # define do_div(n,base) ({					\
 	uint32_t __base = (base);				\
 	uint32_t __rem;						\
+	BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));		\
 	__rem = ((uint64_t)(n)) % __base;			\
 	(n) = ((uint64_t)(n)) / __base;				\
 	__rem;							\
@@ -40,6 +42,7 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
 # define do_div(n,base) ({				\
 	uint32_t __base = (base);			\
 	uint32_t __rem;					\
+	BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));	\
 	(void)(((typeof((n)) *)0) == ((uint64_t *)0));	\
 	if (likely(((n) >> 32) == 0)) {			\
 		__rem = (uint32_t)(n) % __base;		\
-- 
1.8.4

Re: [PATCH] core: Catch overflows in do_div() function

[Joe Perches]

On Tue, 2013-10-08 at 09:10 -0700, Anatol Pomozov wrote:
> If second parameter passed to this function was 64 then it silently
> truncates to 32 bits. Catch such situation.
[]
> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
[]
> @@ -25,6 +26,7 @@
>  # define do_div(n,base) ({					\
>  	uint32_t __base = (base);				\
>  	uint32_t __rem;						\
> +	BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));		\

I think this would be better as a BUILD_BUG_ON

Re: [PATCH] core: Catch overflows in do_div() function

[Richard Weinberger]

On Tue, Oct 8, 2013 at 6:18 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2013-10-08 at 09:10 -0700, Anatol Pomozov wrote:
>> If second parameter passed to this function was 64 then it silently
>> truncates to 32 bits. Catch such situation.
> []
>> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
> []
>> @@ -25,6 +26,7 @@
>>  # define do_div(n,base) ({                                   \
>>       uint32_t __base = (base);                               \
>>       uint32_t __rem;                                         \
>> +     BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));          \
>
> I think this would be better as a BUILD_BUG_ON

No. BUILD_BUG_ON works only for constants.

Anatol, have you tested whether your change increases the kernel size?

-- 
Thanks,
//richard

Re: [PATCH] core: Catch overflows in do_div() function

[Joe Perches]

On Tue, 2013-10-08 at 18:45 +0200, Richard Weinberger wrote:
> On Tue, Oct 8, 2013 at 6:18 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2013-10-08 at 09:10 -0700, Anatol Pomozov wrote:
> >> If second parameter passed to this function was 64 then it silently
> >> truncates to 32 bits. Catch such situation.
> > []
> >> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
> > []
> >> @@ -25,6 +26,7 @@
> >>  # define do_div(n,base) ({                                   \
> >>       uint32_t __base = (base);                               \
> >>       uint32_t __rem;                                         \
> >> +     BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));          \
> >
> > I think this would be better as a BUILD_BUG_ON
> 
> No. BUILD_BUG_ON works only for constants.

Add __builtin_constant_p(base).

Re: [PATCH] core: Catch overflows in do_div() function

[Anatol Pomozov]

Hi

On Tue, Oct 8, 2013 at 9:45 AM, Richard Weinberger
<richard.weinberger@gmail.com> wrote:
> On Tue, Oct 8, 2013 at 6:18 PM, Joe Perches <joe@perches.com> wrote:
>> On Tue, 2013-10-08 at 09:10 -0700, Anatol Pomozov wrote:
>>> If second parameter passed to this function was 64 then it silently
>>> truncates to 32 bits. Catch such situation.
>> []
>>> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
>> []
>>> @@ -25,6 +26,7 @@
>>>  # define do_div(n,base) ({                                   \
>>>       uint32_t __base = (base);                               \
>>>       uint32_t __rem;                                         \
>>> +     BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));          \
>>
>> I think this would be better as a BUILD_BUG_ON
>
> No. BUILD_BUG_ON works only for constants.

BUILD_BUG_ON might actually work. In case if 'base' is const it will
check if it fits 32 bits. As far as I see all such usages (when 'base'
is const) are fine. In case if 'base' is 64 bit variable the
compilation fails.

Comparing with previous patch (without "&& base >= (1UL<<32)") it
eliminates warnings in situations when we pass small constants as long
(dozens of such places in HEAD).

Looking at the cases when we use do_div() I see that in many cases we
pass "long" as a second parameter (see __setup_per_zone_wmarks). If we
replace it with div64_s64() we force to use 64 bit arithmetic. But on
32bit platform "long" is 32bit and using div64_s64() here is
redundant. Wouldn't it be better if do_div() would handle this
situation and called required functions based on a) current
architecture b) size of base/n parameters. Something like this
(completely untested and we need __div64_64 on 32 bit platform):

--- a/include/asm-generic/div64.h
+++ b/include/asm-generic/div64.h
@@ -22,12 +22,12 @@

 #if BITS_PER_LONG == 64

-# define do_div(n,base) ({                                     \
-       uint32_t __base = (base);                               \
-       uint32_t __rem;                                         \
-       __rem = ((uint64_t)(n)) % __base;                       \
-       (n) = ((uint64_t)(n)) / __base;                         \
-       __rem;                                                  \
+# define do_div(n,base) ({             \
+       typeof(base) __base = (base);   \
+       typeof(base) __rem;             \
+       __rem = (n) % __base;           \
+       (n) = (n) / __base;             \
+       __rem;                          \
  })

 #elif BITS_PER_LONG == 32
@@ -37,16 +37,20 @@ extern uint32_t __div64_32(uint64_t *dividend,
uint32_t divisor);
 /* The unnecessary pointer compare is there
  * to check for type safety (n must be 64bit)
  */
-# define do_div(n,base) ({                             \
-       uint32_t __base = (base);                       \
-       uint32_t __rem;                                 \
-       (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
-       if (likely(((n) >> 32) == 0)) {                 \
-               __rem = (uint32_t)(n) % __base;         \
-               (n) = (uint32_t)(n) / __base;           \
-       } else                                          \
-               __rem = __div64_32(&(n), __base);       \
-       __rem;                                          \
+# define do_div(n,base) ({                                     \
+       typeof(base) __base = (base);                           \
+       typeof(base) __rem;                                     \
+       (void)(((typeof((n)) *)0) == ((uint64_t *)0));          \
+       if (sizeof(__base) <= 4 || (__builtin_constant_p(__base) &&
__base < (1ULL<<32)) ) { \
+               if (likely(((n) >> 32) == 0)) {                 \
+                       __rem = (uint32_t)(n) % __base;         \
+                       (n) = (uint32_t)(n) / __base;           \
+               } if (sizeof(base) <= 4)                        \
+                       __rem = __div64_32(&(n), __base);       \
+       } else {                                                \
+               __rem = __div64_64(&(n), __base);               \
+       }                                                       \
+       __rem;                                                  \
  })

 #else /* BITS_PER_LONG == ?? */

Re: [PATCH] core: Catch overflows in do_div() function

[Anatol Pomozov]

Hi

On Tue, Oct 8, 2013 at 10:28 AM, Anatol Pomozov
<anatol.pomozov@gmail.com> wrote:
> Hi
>
> On Tue, Oct 8, 2013 at 9:45 AM, Richard Weinberger
> <richard.weinberger@gmail.com> wrote:
>> On Tue, Oct 8, 2013 at 6:18 PM, Joe Perches <joe@perches.com> wrote:
>>> On Tue, 2013-10-08 at 09:10 -0700, Anatol Pomozov wrote:
>>>> If second parameter passed to this function was 64 then it silently
>>>> truncates to 32 bits. Catch such situation.
>>> []
>>>> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
>>> []
>>>> @@ -25,6 +26,7 @@
>>>>  # define do_div(n,base) ({                                   \
>>>>       uint32_t __base = (base);                               \
>>>>       uint32_t __rem;                                         \
>>>> +     BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));          \
>>>
>>> I think this would be better as a BUILD_BUG_ON
>>
>> No. BUILD_BUG_ON works only for constants.
>
> BUILD_BUG_ON might actually work. In case if 'base' is const it will
> check if it fits 32 bits. As far as I see all such usages (when 'base'
> is const) are fine. In case if 'base' is 64 bit variable the
> compilation fails.
>
> Comparing with previous patch (without "&& base >= (1UL<<32)") it
> eliminates warnings in situations when we pass small constants as long
> (dozens of such places in HEAD).
>
> Looking at the cases when we use do_div() I see that in many cases we
> pass "long" as a second parameter (see __setup_per_zone_wmarks). If we
> replace it with div64_s64() we force to use 64 bit arithmetic. But on
> 32bit platform "long" is 32bit and using div64_s64() here is
> redundant. Wouldn't it be better if do_div() would handle this
> situation and called required functions based on a) current
> architecture b) size of base/n parameters. Something like this
> (completely untested and we need __div64_64 on 32 bit platform):
>
> --- a/include/asm-generic/div64.h
> +++ b/include/asm-generic/div64.h
> @@ -22,12 +22,12 @@
>
>  #if BITS_PER_LONG == 64
>
> -# define do_div(n,base) ({                                     \
> -       uint32_t __base = (base);                               \
> -       uint32_t __rem;                                         \
> -       __rem = ((uint64_t)(n)) % __base;                       \
> -       (n) = ((uint64_t)(n)) / __base;                         \
> -       __rem;                                                  \
> +# define do_div(n,base) ({             \
> +       typeof(base) __base = (base);   \

Documentation says typeof() has side-effects and can be used on
arithmetic types only. :(

> +       typeof(base) __rem;             \
> +       __rem = (n) % __base;           \
> +       (n) = (n) / __base;             \
> +       __rem;                          \
>   })
>
>  #elif BITS_PER_LONG == 32
> @@ -37,16 +37,20 @@ extern uint32_t __div64_32(uint64_t *dividend,
> uint32_t divisor);
>  /* The unnecessary pointer compare is there
>   * to check for type safety (n must be 64bit)
>   */
> -# define do_div(n,base) ({                             \
> -       uint32_t __base = (base);                       \
> -       uint32_t __rem;                                 \
> -       (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
> -       if (likely(((n) >> 32) == 0)) {                 \
> -               __rem = (uint32_t)(n) % __base;         \
> -               (n) = (uint32_t)(n) / __base;           \
> -       } else                                          \
> -               __rem = __div64_32(&(n), __base);       \
> -       __rem;                                          \
> +# define do_div(n,base) ({                                     \
> +       typeof(base) __base = (base);                           \
> +       typeof(base) __rem;                                     \
> +       (void)(((typeof((n)) *)0) == ((uint64_t *)0));          \
> +       if (sizeof(__base) <= 4 || (__builtin_constant_p(__base) &&
> __base < (1ULL<<32)) ) { \
> +               if (likely(((n) >> 32) == 0)) {                 \
> +                       __rem = (uint32_t)(n) % __base;         \
> +                       (n) = (uint32_t)(n) / __base;           \
> +               } if (sizeof(base) <= 4)                        \
> +                       __rem = __div64_32(&(n), __base);       \
> +       } else {                                                \
> +               __rem = __div64_64(&(n), __base);               \
> +       }                                                       \
> +       __rem;                                                  \
>   })
>
>  #else /* BITS_PER_LONG == ?? */

Re: [PATCH] core: Catch overflows in do_div() function

[Eric Dumazet]

On Tue, 2013-10-08 at 09:10 -0700, Anatol Pomozov wrote:
> If second parameter passed to this function was 64 then it silently
> truncates to 32 bits. Catch such situation.
> 
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> ---
>  include/asm-generic/div64.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
> index 8f4e319..84339a0 100644
> --- a/include/asm-generic/div64.h
> +++ b/include/asm-generic/div64.h
> @@ -17,6 +17,7 @@
>   *       beware of side effects!
>   */
>  
> +#include <linux/bug.h>
>  #include <linux/types.h>
>  #include <linux/compiler.h>
>  
> @@ -25,6 +26,7 @@
>  # define do_div(n,base) ({					\
>  	uint32_t __base = (base);				\
>  	uint32_t __rem;						\
> +	BUG_ON(sizeof(base) > 4 && base >= (1UL<<32));		\

Problem is about

uint32_t __base = (base);

This was designed to avoid "base" being evaluated twice, as in

do_div(X, ++Y);

So I guess you need something to keep this in place.