Age | Commit message (Collapse) | Author | Files | Lines |
|
To simplify the function body, invert the if() statement, returning
earlier.
Since we already checked there is enough data in the iovec buffer,
simply add an assert() call to consume the bytes_read variable.
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
We can't know the caller read enough data in the memory pointed
by ext_hdr to cast it as a ip6_ext_hdr_routing.
Declare rt_hdr on the stack and fill it again from the iovec.
Since we already checked there is enough data in the iovec buffer,
simply add an assert() call to consume the bytes_read variable.
This fix a 2 bytes buffer overrun in eth_parse_ipv6_hdr() reported
by QEMU fuzzer:
$ cat << EOF | ./qemu-system-i386 -M pc-q35-5.0 \
-accel qtest -monitor none \
-serial none -nographic -qtest stdio
outl 0xcf8 0x80001010
outl 0xcfc 0xe1020000
outl 0xcf8 0x80001004
outw 0xcfc 0x7
write 0x25 0x1 0x86
write 0x26 0x1 0xdd
write 0x4f 0x1 0x2b
write 0xe1020030 0x4 0x190002e1
write 0xe102003a 0x2 0x0807
write 0xe1020048 0x4 0x12077cdd
write 0xe1020400 0x4 0xba077cdd
write 0xe1020420 0x4 0x190002e1
write 0xe1020428 0x4 0x3509d807
write 0xe1020438 0x1 0xe2
EOF
=================================================================
==2859770==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdef904902 at pc 0x561ceefa78de bp 0x7ffdef904820 sp 0x7ffdef904818
READ of size 1 at 0x7ffdef904902 thread T0
#0 0x561ceefa78dd in _eth_get_rss_ex_dst_addr net/eth.c:410:17
#1 0x561ceefa41fb in eth_parse_ipv6_hdr net/eth.c:532:17
#2 0x561cef7de639 in net_tx_pkt_parse_headers hw/net/net_tx_pkt.c:228:14
#3 0x561cef7dbef4 in net_tx_pkt_parse hw/net/net_tx_pkt.c:273:9
#4 0x561ceec29f22 in e1000e_process_tx_desc hw/net/e1000e_core.c:730:29
#5 0x561ceec28eac in e1000e_start_xmit hw/net/e1000e_core.c:927:9
#6 0x561ceec1baab in e1000e_set_tdt hw/net/e1000e_core.c:2444:9
#7 0x561ceebf300e in e1000e_core_write hw/net/e1000e_core.c:3256:9
#8 0x561cef3cd4cd in e1000e_mmio_write hw/net/e1000e.c:110:5
Address 0x7ffdef904902 is located in stack of thread T0 at offset 34 in frame
#0 0x561ceefa320f in eth_parse_ipv6_hdr net/eth.c:486
This frame has 1 object(s):
[32, 34) 'ext_hdr' (line 487) <== Memory access at offset 34 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow net/eth.c:410:17 in _eth_get_rss_ex_dst_addr
Shadow bytes around the buggy address:
0x10003df188d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df188e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df188f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18910: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
=>0x10003df18920:[02]f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Stack left redzone: f1
Stack right redzone: f3
==2859770==ABORTING
Add the corresponding qtest case with the fuzzer reproducer.
FWIW GCC 11 similarly reported:
net/eth.c: In function 'eth_parse_ipv6_hdr':
net/eth.c:410:15: error: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds]
410 | if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
| ~~~~~^~~~~~~
net/eth.c:485:24: note: while referencing 'ext_hdr'
485 | struct ip6_ext_hdr ext_hdr;
| ^~~~~~~
net/eth.c:410:38: error: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds]
410 | if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
| ~~~~~^~~~~~~~~
net/eth.c:485:24: note: while referencing 'ext_hdr'
485 | struct ip6_ext_hdr ext_hdr;
| ^~~~~~~
Cc: qemu-stable@nongnu.org
Buglink: https://bugs.launchpad.net/qemu/+bug/1879531
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Fixes: eb700029c78 ("net_pkt: Extend packet abstraction as required by e1000e functionality")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
We want to check fields from ip6_ext_hdr_routing structure
and if correct read the full in6_address. Let's directly check
if our iovec contains enough data for everything, else return
early.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
The 'offset' argument represents the offset to the ip6_ext_hdr
header, rename it as 'ext_hdr_offset'.
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
The length field is already contained in the ip6_ext_hdr structure.
Check it direcly in eth_parse_ipv6_hdr() before calling
_eth_get_rss_ex_dst_addr(), which gets a bit simplified.
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
The in6_address comes after the ip6_ext_hdr_routing header,
not after the ip6_ext_hdr one. Fix the offset.
Cc: qemu-stable@nongnu.org
Reported-by: Stefano Garzarella <sgarzare@redhat.com>
Fixes: eb700029c78 ("net_pkt: Extend packet abstraction as required by e1000e functionality")
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
Add a helper to pad a short Ethernet frame to the minimum required
length, which can be used by backends' code.
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
eth_get_gso_type() routine returns segmentation offload type based on
L3 protocol type. It calls g_assert_not_reached if L3 protocol is
unknown, making the following return statement unreachable. Remove the
g_assert call, it maybe triggered by a guest user.
Reported-by: Gaoning Pan <pgn@zju.edu.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
No header includes qemu-common.h after this commit, as prescribed by
qemu-common.h's file comment.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190523143508.25387-5-armbru@redhat.com>
[Rebased with conflicts resolved automatically, except for
include/hw/arm/xlnx-zynqmp.h hw/arm/nrf51_soc.c hw/arm/msf2-soc.c
block/qcow2-refcount.c block/qcow2-cluster.c block/qcow2-cache.c
target/arm/cpu.h target/lm32/cpu.h target/m68k/cpu.h target/mips/cpu.h
target/moxie/cpu.h target/nios2/cpu.h target/openrisc/cpu.h
target/riscv/cpu.h target/tilegx/cpu.h target/tricore/cpu.h
target/unicore32/cpu.h target/xtensa/cpu.h; bsd-user/main.c and
net/tap-bsd.c fixed up]
|
|
So we have sizeof(struct in6_address) != sizeof(uintptr_t)
and Clang > Coverity on this, see 4555ca6816c :)
net/eth.c:426:30: warning: The code calls sizeof() on a pointer type. This can produce an unexpected result
return bytes_read == sizeof(dst_addr);
^ ~~~~~~~~~~
net/eth.c:475:34: warning: The code calls sizeof() on a pointer type. This can produce an unexpected result
return bytes_read == sizeof(src_addr);
^ ~~~~~~~~~~
Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
|
|
Make VLAN stripping functions return number of bytes
copied to given Ethernet header buffer.
This information should be used to re-compose
packet IOV after VLAN stripping.
Cc: qemu-stable@nongnu.org
Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
Coverity reports a "suspicious sizeof" which is indeed wrong.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
This is not dereferencing the pointer, and instead checking only
the value of the pointer.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
This patch extends the TX/RX packet abstractions with features that will
be used by the e1000e device implementation.
Changes are:
1. Support iovec lists for RX buffers
2. Deeper RX packets parsing
3. Loopback option for TX packets
4. Extended VLAN headers handling
5. RSS processing for RX packets
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
|
|
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1454089805-5470-11-git-send-email-peter.maydell@linaro.org
|
|
The macro g_assert_not_reached is a better self documenting replacement
for assert(0) or assert(false).
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
|
|
Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
Signed-off-by: Yan Vugenfirer <yan@daynix.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
|