[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-commits] [qemu/qemu] af7745: net: eth: Add a helper to pad a short
|
From: |
Peter Maydell |
|
Subject: |
[Qemu-commits] [qemu/qemu] af7745: net: eth: Add a helper to pad a short Ethernet frame |
|
Date: |
Mon, 22 Mar 2021 07:14:35 -0700 |
Branch: refs/heads/master
Home: https://github.com/qemu/qemu
Commit: af774513f7d646badfdb5b686650254f7f08af6b
https://github.com/qemu/qemu/commit/af774513f7d646badfdb5b686650254f7f08af6b
Author: Bin Meng <bmeng.cn@gmail.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M include/net/eth.h
M net/eth.c
Log Message:
-----------
net: eth: Add a helper to pad a short Ethernet frame
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>
Commit: 935344bed6769d6bcb74c6d992818929a6ccb35b
https://github.com/qemu/qemu/commit/935344bed6769d6bcb74c6d992818929a6ccb35b
Author: Bin Meng <bmeng.cn@gmail.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M include/net/net.h
Log Message:
-----------
net: Add a 'do_not_pad" to NetClientState
This adds a flag in NetClientState, so that a net client can tell
its peer that the packets do not need to be padded to the minimum
size of an Ethernet frame (60 bytes) before sending to it.
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>
Commit: 969e50b61a285b0cc8dea6d4d2ade3f758d5ecc7
https://github.com/qemu/qemu/commit/969e50b61a285b0cc8dea6d4d2ade3f758d5ecc7
Author: Bin Meng <bmeng.cn@gmail.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/slirp.c
M net/tap-win32.c
M net/tap.c
Log Message:
-----------
net: Pad short frames to minimum size before sending from SLiRP/TAP
The minimum Ethernet frame length is 60 bytes. For short frames with
smaller length like ARP packets (only 42 bytes), on a real world NIC
it can choose either padding its length to the minimum required 60
bytes, or sending it out directly to the wire. Such behavior can be
hardcoded or controled by a register bit. Similarly on the receive
path, NICs can choose either dropping such short frames directly or
handing them over to software to handle.
On the other hand, for the network backends like SLiRP/TAP, they
don't expose a way to control the short frame behavior. As of today
they just send/receive data from/to the other end connected to them,
which means any sized packet is acceptable. So they can send and
receive short frames without any problem. It is observed that ARP
packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
these ARP packets to the other end which might be a NIC model that
does not allow short frames to pass through.
To provide better compatibility, for packets sent from QEMU network
backends like SLiRP/TAP, we change to pad short frames before sending
it out to the other end, if the other end does not forbid it via the
nc->do_not_pad flag. This ensures a backend as an Ethernet sender
does not violate the spec. But with this change, the behavior of
dropping short frames from SLiRP/TAP interfaces in the NIC model
cannot be emulated because it always receives a packet that is spec
complaint. The capability of sending short frames from NIC models is
still supported and short frames can still pass through SLiRP/TAP.
This commit should be able to fix the issue as reported with some
NIC models before, that ARP requests get dropped, preventing the
guest from becoming visible on the network. It was workarounded in
these NIC models on the receive path, that when a short frame is
received, it is padded up to 60 bytes.
The following 2 commits seem to be the one to workaround this issue
in e1000 and vmxenet3 before, and should probably be reverted.
commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)")
commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 bytes)")
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>
Commit: d4c6293041ee7941191a91e4ca2d2af4b0959599
https://github.com/qemu/qemu/commit/d4c6293041ee7941191a91e4ca2d2af4b0959599
Author: Bin Meng <bmeng.cn@gmail.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M hw/net/virtio-net.c
Log Message:
-----------
hw/net: virtio-net: Initialize nc->do_not_pad to true
For virtio-net, there is no need to pad the Ethernet frame size to
60 bytes before sending to it.
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Commit: 9162ed664926fe6e8dfa2f43e152ab088b5369ed
https://github.com/qemu/qemu/commit/9162ed664926fe6e8dfa2f43e152ab088b5369ed
Author: Lukas Straub <lukasstraub2@web.de>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/colo-compare.c
Log Message:
-----------
net/colo-compare.c: Fix memory leak for non-tcp packet
Additional to removing the packet from the secondary queue,
we also need to free it.
Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Commit: 739128e43b6da3d0a48ec8551d94909dc5a8f3bc
https://github.com/qemu/qemu/commit/739128e43b6da3d0a48ec8551d94909dc5a8f3bc
Author: Lukas Straub <lukasstraub2@web.de>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/colo-compare.c
Log Message:
-----------
net/colo-compare.c: Optimize removal of secondary packet
g_queue_remove needs to look up the list entry first, but we
already have it as result and can remove it directly with
g_queue_delete_link.
Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Commit: b565b44ec202fbe52a017273319db83f067fe574
https://github.com/qemu/qemu/commit/b565b44ec202fbe52a017273319db83f067fe574
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/eth.c
Log Message:
-----------
net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr()
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>
Commit: 38462440ca22a8ead2ec1e98ef3c45e264fa6f60
https://github.com/qemu/qemu/commit/38462440ca22a8ead2ec1e98ef3c45e264fa6f60
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/eth.c
Log Message:
-----------
net/eth: Simplify _eth_get_rss_ex_dst_addr()
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>
Commit: dbd8d3f959e5ba9b1804a5c99c7f8af42d96809b
https://github.com/qemu/qemu/commit/dbd8d3f959e5ba9b1804a5c99c7f8af42d96809b
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/eth.c
Log Message:
-----------
net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument
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>
Commit: 6f10f77dcdbc151217d19229d9aeeb93c9c1c408
https://github.com/qemu/qemu/commit/6f10f77dcdbc151217d19229d9aeeb93c9c1c408
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/eth.c
Log Message:
-----------
net/eth: Check size earlier in _eth_get_rss_ex_dst_addr()
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>
Commit: ef763586c943815eb0836c54c207ce8572e176a7
https://github.com/qemu/qemu/commit/ef763586c943815eb0836c54c207ce8572e176a7
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/eth.c
Log Message:
-----------
net/eth: Check iovec has enough data earlier
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>
Commit: 7d6a4f123e00c9dbd40867ae1a650a4fd0bc4a3d
https://github.com/qemu/qemu/commit/7d6a4f123e00c9dbd40867ae1a650a4fd0bc4a3d
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M MAINTAINERS
M net/eth.c
A tests/qtest/fuzz-e1000e-test.c
M tests/qtest/meson.build
Log Message:
-----------
net/eth: Read ip6_ext_hdr_routing buffer before accessing it
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>
Commit: c7274b5ef43614dd133daec1e2018f71d8744088
https://github.com/qemu/qemu/commit/c7274b5ef43614dd133daec1e2018f71d8744088
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M net/eth.c
Log Message:
-----------
net/eth: Add an assert() and invert if() statement to simplify code
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>
Commit: b184750926812cb78ac0caf4c4b2b13683b5bde3
https://github.com/qemu/qemu/commit/b184750926812cb78ac0caf4c4b2b13683b5bde3
Author: Peter Maydell <peter.maydell@linaro.org>
Date: 2021-03-22 (Mon, 22 Mar 2021)
Changed paths:
M MAINTAINERS
M hw/net/virtio-net.c
M include/net/eth.h
M include/net/net.h
M net/colo-compare.c
M net/eth.c
M net/slirp.c
M net/tap-win32.c
M net/tap.c
A tests/qtest/fuzz-e1000e-test.c
M tests/qtest/meson.build
Log Message:
-----------
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into
staging
# gpg: Signature made Mon 22 Mar 2021 09:35:08 GMT
# gpg: using RSA key EF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat)
<jasowang@redhat.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F 3562 EF04 965B 398D 6211
* remotes/jasowang/tags/net-pull-request:
net/eth: Add an assert() and invert if() statement to simplify code
net/eth: Read ip6_ext_hdr_routing buffer before accessing it
net/eth: Check iovec has enough data earlier
net/eth: Check size earlier in _eth_get_rss_ex_dst_addr()
net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument
net/eth: Simplify _eth_get_rss_ex_dst_addr()
net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr()
net/colo-compare.c: Optimize removal of secondary packet
net/colo-compare.c: Fix memory leak for non-tcp packet
hw/net: virtio-net: Initialize nc->do_not_pad to true
net: Pad short frames to minimum size before sending from SLiRP/TAP
net: Add a 'do_not_pad" to NetClientState
net: eth: Add a helper to pad a short Ethernet frame
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Compare: https://github.com/qemu/qemu/compare/f0f20022a0c7...b18475092681