| Message ID | 20230118152329.877-1-dthaler1968@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf, docs: Fix modulo zero, division by zero, overflow, and underflow | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain }} |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for ShellCheck |
| bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for build for aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-4 | success | Logs for build for aarch64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-5 | fail | Logs for build for s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-6 | success | Logs for build for x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-7 | success | Logs for build for x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-8 | success | Logs for llvm-toolchain |
| bpf/vmtest-bpf-next-VM_Test-9 | success | Logs for set-matrix |
| netdev/tree_selection | success | Not a local patch |
On 1/18/23 4:23 PM, dthaler1968@googlemail.com wrote: > From: Dave Thaler <dthaler@microsoft.com> > > Fix modulo zero, division by zero, overflow, and underflow. > Also clarify how a negative immediate value is used in unsigned division > > Changes from last submission: addressed conversion comment from > Jose. > > Signed-off-by: Dave Thaler <dthaler@microsoft.com> > --- > Documentation/bpf/instruction-set.rst | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst > index e672d5ec6cc..f79dae527ad 100644 > --- a/Documentation/bpf/instruction-set.rst > +++ b/Documentation/bpf/instruction-set.rst > @@ -99,19 +99,26 @@ code value description > BPF_ADD 0x00 dst += src > BPF_SUB 0x10 dst -= src > BPF_MUL 0x20 dst \*= src > -BPF_DIV 0x30 dst /= src > +BPF_DIV 0x30 dst = (src != 0) ? (dst / src) : 0 > BPF_OR 0x40 dst \|= src > BPF_AND 0x50 dst &= src > BPF_LSH 0x60 dst <<= src > BPF_RSH 0x70 dst >>= src > BPF_NEG 0x80 dst = ~src > -BPF_MOD 0x90 dst %= src > +BPF_MOD 0x90 dst = (src != 0) ? (dst % src) : dst > BPF_XOR 0xa0 dst ^= src > BPF_MOV 0xb0 dst = src > BPF_ARSH 0xc0 sign extending shift right > BPF_END 0xd0 byte swap operations (see `Byte swap instructions`_ below) > ======== ===== ========================================================== > > +Underflow and overflow are allowed during arithmetic operations, > +meaning the 64-bit or 32-bit value will wrap. If > +eBPF program execution would result in division by zero, > +the destination register is instead set to zero. > +If execution would result in modulo by zero, > +the destination register is instead left unchanged. Looks good to go with one small nit for the previous sentence which could be misinterpreted. The rewrites from verifier for modulo op are: mod32: mod64: (16) if w0 == 0x0 goto pc+2 (15) if r0 == 0x0 goto pc+1 (9c) w1 %= w0 (9f) r1 %= r0 (05) goto pc+1 (bc) w1 = w1 So for BPF_ALU as 32-bit op, it is expected that the result is 32-bit value, too. So for 32-bit op the destination register is truncated to 32-bit value. (Related commit 9b00f1b78809 ("bpf: Fix truncation handling for mod32 dst reg wrt zero")). > ``BPF_ADD | BPF_X | BPF_ALU`` means:: > > dst_reg = (u32) dst_reg + (u32) src_reg; > @@ -128,6 +135,11 @@ BPF_END 0xd0 byte swap operations (see `Byte swap instructions`_ below) > > dst_reg = dst_reg ^ imm32 > > +Also note that the division and modulo operations are unsigned. > +Thus, for `BPF_ALU`, 'imm' is first interpreted as an unsigned > +32-bit value, whereas for `BPF_ALU64`, 'imm' is first sign extended > +to 64 bits and the result interpreted as an unsigned 64-bit value. > +There are no instructions for signed division or modulo. > > Byte swap instructions > ~~~~~~~~~~~~~~~~~~~~~~ >
diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst index e672d5ec6cc..f79dae527ad 100644 --- a/Documentation/bpf/instruction-set.rst +++ b/Documentation/bpf/instruction-set.rst @@ -99,19 +99,26 @@ code value description BPF_ADD 0x00 dst += src BPF_SUB 0x10 dst -= src BPF_MUL 0x20 dst \*= src -BPF_DIV 0x30 dst /= src +BPF_DIV 0x30 dst = (src != 0) ? (dst / src) : 0 BPF_OR 0x40 dst \|= src BPF_AND 0x50 dst &= src BPF_LSH 0x60 dst <<= src BPF_RSH 0x70 dst >>= src BPF_NEG 0x80 dst = ~src -BPF_MOD 0x90 dst %= src +BPF_MOD 0x90 dst = (src != 0) ? (dst % src) : dst BPF_XOR 0xa0 dst ^= src BPF_MOV 0xb0 dst = src BPF_ARSH 0xc0 sign extending shift right BPF_END 0xd0 byte swap operations (see `Byte swap instructions`_ below) ======== ===== ========================================================== +Underflow and overflow are allowed during arithmetic operations, +meaning the 64-bit or 32-bit value will wrap. If +eBPF program execution would result in division by zero, +the destination register is instead set to zero. +If execution would result in modulo by zero, +the destination register is instead left unchanged. + ``BPF_ADD | BPF_X | BPF_ALU`` means:: dst_reg = (u32) dst_reg + (u32) src_reg; @@ -128,6 +135,11 @@ BPF_END 0xd0 byte swap operations (see `Byte swap instructions`_ below) dst_reg = dst_reg ^ imm32 +Also note that the division and modulo operations are unsigned. +Thus, for `BPF_ALU`, 'imm' is first interpreted as an unsigned +32-bit value, whereas for `BPF_ALU64`, 'imm' is first sign extended +to 64 bits and the result interpreted as an unsigned 64-bit value. +There are no instructions for signed division or modulo. Byte swap instructions ~~~~~~~~~~~~~~~~~~~~~~