Re: [PATCH bpf-next 1/2] bpf: Fix a sdiv overflow issue

[Yonghong Song <yonghong.song@linux.dev>]

On 9/11/24 8:52 AM, Alexei Starovoitov wrote:
> On Tue, Sep 10, 2024 at 9:40 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Zac Ecob reported a problem where a bpf program may cause kernel crash due
>> to the following error:
>>    Oops: divide error: 0000 [#1] PREEMPT SMP KASAN PTI
>>
>> The failure is due to the below signed divide:
>>    LLONG_MIN/-1 where LLONG_MIN equals to -9,223,372,036,854,775,808.
>> LLONG_MIN/-1 is supposed to give a positive number 9,223,372,036,854,775,808,
>> but it is impossible since for 64-bit system, the maximum positive
>> number is 9,223,372,036,854,775,807. On x86_64, LLONG_MIN/-1 will
>> cause a kernel exception. On arm64, the result for LLONG_MIN/-1 is
>> LLONG_MIN.
>>
>> So for 64-bit signed divide (sdiv), some additional insns are patched
>> to check LLONG_MIN/-1 pattern. If such a pattern does exist, the result
>> will be LLONG_MIN. Otherwise, it follows normal sdiv operation.
>>
>>    [1] https://lore.kernel.org/bpf/tPJLTEh7S_DxFEqAI2Ji5MBSoZVg7_G-Py2iaZpAaWtM961fFTWtsnlzwvTbzBzaUzwQAoNATXKUlt0LZOFgnDcIyKCswAnAGdUF3LBrhGQ=@protonmail.com/
>>
>> Reported-by: Zac Ecob <zacecob@protonmail.com>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   kernel/bpf/verifier.c | 29 ++++++++++++++++++++++++++---
>>   1 file changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index f35b80c16cda..d77f1a05a065 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -20506,6 +20506,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
>>                      insn->code == (BPF_ALU | BPF_DIV | BPF_X)) {
>>                          bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;
>>                          bool isdiv = BPF_OP(insn->code) == BPF_DIV;
>> +                       bool is_sdiv64 = is64 && isdiv && insn->off == 1;
> I suspect signed mod has the same issue.

Okay, you are correct. 64bit mod has the same problem.

On x86_64,

$ cat t10.c
#include <stdio.h>
#include <limits.h>
int main(void) {
   volatile long long a = LLONG_MIN;
   volatile long long b = -1;
   printf("a%%b = %lld\n", a%b);
   return 0;
}
$ gcc -O2 t10.c && ./a.out
Floating point exception (core dumped)

I tried the same thing with bpf inline asm and the kernel crashed.

On arm64,
the compiled binary can run successfully and the result is
a%b = 0

> Also is it only a 64-bit ? 32-bit sdiv/smod are also affected, no?

Yes, 32bit sdiv/smod also affect.

On x86,

$ cat t11.c
#include <stdio.h>
#include <limits.h>
int main(void) {
   volatile int a = INT_MIN;
   volatile int b = -1;
   printf("a/b = %d\n", a/b);
   return 0;
}
$ gcc -O2 t11.c && ./a.out
Floating point exception (core dumped)

On arm64,
   a/b = -2147483648  // INT_MIN
   a%b = 0

>
> pw-bot: cr