lkml.org 
[lkml]   [2020]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: Re:Re: [PATCH] powerpc: Fix a bug in div64 32 if divisor is zero
Date
>> >In generic version in lib/math/div64.c, there is no checking of 'base' 
>> >either.
>> >Do we really want to add this check in the powerpc version only ?
>>
>> >The only user of __div64_32() is do_div() in
>> >include/asm-generic/div64.h. Wouldn't it be better to do the check there ?
>>
>> >Christophe
>>
>> Yet, I have noticed that there is no checking of 'base' in these functions.
>> But I am not sure how to check is better.As we know that the result is
>> undefined when divisor is zero. It maybe good to print error and dump stack.
>> Let the process to know that the divisor is zero by sending SIGFPE.
>>
>> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
>> index a3b98c86f077..161c656ee3ee 100644
>> --- a/include/asm-generic/div64.h
>> +++ b/include/asm-generic/div64.h
>> @@ -43,6 +43,11 @@
>> # define do_div(n,base) ({ \
>> uint32_t __base = (base); \
>> uint32_t __rem; \
>> + if (unlikely(base == 0)) { \
>> + pr_err("do_div base=%d\n",base); \
>> + dump_stack(); \
>> + force_sig(SIGFPE); \
>> + }
>>

> I suspect this will generate a strong reaction. SIGFPE is for user space
> instruction attempting a division by zero. A division by zero in the
> kernel is a kernel bug, period, and you don't want to kill a user
> process for this reason.

> If it happens in an interrupt, the context of the kernel may not even be
> related to the current process.

> Many other architectures (x86 for example) already trigger an exception
> on a division by zero but the handler will find that the exception
> happened in kernel context and generate an Oops, not raise a signal in a
> (possibly innocent) userland process.

OK. So just don't touch do_div functions in include/asm-generic/div64.h
But for powerpc it can not trigger an exception when divisor is 0 in __div64_32.


So the patch as below is still useful for powerpc. If this patch looks good for
you, please help to review. I will send the new patch later.

Thanks for your reply.

diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S
index 4354928ed62e..1d3561cf16fa 100644
--- a/arch/powerpc/boot/div64.S
+++ b/arch/powerpc/boot/div64.S
@@ -13,8 +13,10 @@

.globl __div64_32
.globl __div64_32
__div64_32:
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,7 +54,7 @@ __div64_32:
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero

/*
* Extended precision shifts.
diff --git a/arch/powerpc/lib/div64.S b/arch/powerpc/lib/div64.S
index 3d5426e7dcc4..570774d9782d 100644
--- a/arch/powerpc/lib/div64.S
+++ b/arch/powerpc/lib/div64.S
@@ -13,8 +13,10 @@
#include <asm/processor.h>

_GLOBAL(__div64_32)
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,4 +54,4 @@ _GLOBAL(__div64_32)
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
Guohua

\
 
 \ /
  Last update: 2020-08-24 15:26    [W:0.096 / U:9.884 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site