On 21.01.2019 14:47, Yuriy M. Kaminskiy wrote:
lan78xx.c:rx_submit() allocates space for frame-to-be-received with
netdev_alloc_skb_ip_align(), which misalign start of buffer by 2 bytes
in expectation that frame will start from 14-byte ethernet header, then
ip header; if start of buffer misaligned by 2 bytes, ip header will be
16-byte aligned.
Unfortunately, usb frame that sent by lan78xx starts with another
10-byte header (lan78xx_rx(): rx_cmd_a/rx_cmd_b/rx_cmd_c), *then*
follows ethernet header, and *then* ip header (which ends up being
misaligned).
This issue was noticed on arm platform (where misaligned 32-bit word
access triggers exception and leave traces in /proc/cpu/alignment, see
https://github.com/raspberrypi/linux/issues/2599 ; for me, about any
ipv6 traffic that hits machine - `ping -I eth0 ip6-allnodes`, tcp/udp
packets, etc triggered increase in this counter, with
ip6_datagram_recv_common_ctl, icmpv6_echo_reply, etc as culprit).
If we just allocate skb data without any misalignment tricks, ip header
will end up and at offset 24 (8-byte aligned).
Patch attached; runtime-tested with raspbian fork of stable/4.14.y
[4.14.92] on Raspberry pi 3B+
I quickly reviewed other usbnet drivers for similar problems, it looks like:
1) ax88179_178a.c likely has similar problem (multiple packets in usb
frame with 2-byte packet header, padded at end to 8-byte boundary; if
first packet alignment will be fixed, it will fix all of them) and
contains several potential read buffer overflows with misbehaving or
malicious device.
2) cdc_eem.c likely has similar problem (usb frame contains several
packets with 2-byte header), see eem_rx_fixup()
3) cx82310_eth.c - ditto, see cx82310_rx_fixup()
4) dm9601.c - weird 3-byte header, affected, would need 7 bytes
prepended to compensate, see dm9601_rx_fixup()
5) kalmia.c - likely affected, 6-byte header; no padding/alignment
between packets, so 2nd and following packet may end up misaligned anyway.
6) lg-vl600.c - unsure; multi-packet frames, first packet probably fine,
but no padding/alignment between packets, so 2nd and following packets
may end up misaligned anyway.
7) net1080.c - something weird, probably affected.
8) pegasus.c - (not usbnet based) not sure, but `pegasus->chip ==
0x8513` seems affected (2-byte header prepended to ethernet frame), see
read_bulk_callback()
9) rndis_host.c - unpredictable, packet offset is driven by device?
10) sierra_net.c - ditto
11) smsc75xx.c - 10-byte packet header, affected, padded to 4-bytes
alignment at end;
12) smsc95xx.c [fixed in master]
13) sr9700.c - weird 3-byte packet header, multiply packets; no
alignment between packets, so 2nd and following packet may end up
misaligned anyway.
Given I have no access to hw, I won't try to compose patches for them
(except for not-even-compile-tested patch to ax88179_178a.c as PoC).
P.S. I'm not subscribed, please CC me on reply.
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 0f69b77e8..85e48000f 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1234,6 +1234,9 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
struct ethtool_eee eee_data;
+ /* XXX is this a right place? */
+ set_bit(EVENT_NO_IP_ALIGN, &dev->flags);
+
usbnet_get_endpoints(dev, intf);
tmp16 = (u16 *)buf;
@@ -1383,6 +1386,10 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
pkt_cnt = (u16)rx_hdr;
hdr_off = (u16)(rx_hdr >> 16);
+ if (hdr_off + sizeof(u32)*pkt_cnt > skb->len) {
+ /* XXX misbehaving or malicious device, report? */
+ return 0;
+ }
pkt_hdr = (u32 *)(skb->data + hdr_off);
while (pkt_cnt--) {
@@ -1390,11 +1397,19 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
le32_to_cpus(pkt_hdr);
pkt_len = (*pkt_hdr >> 16) & 0x1fff;
+ if (pkt_len + 2 > skb->len) {
+ /* XXX misbehaving or malicious device, report? */
+ return 0;
+ }
/* Check CRC or runt packet */
if ((*pkt_hdr & AX_RXHDR_CRC_ERR) ||
(*pkt_hdr & AX_RXHDR_DROP_ERR)) {
- skb_pull(skb, (pkt_len + 7) & 0xFFF8);
+ /* XXX actual packet size is (2+pkt_len), but we advance by pkt_len here? */
+ if (skb_pull(skb, (pkt_len + 7) & 0xFFF8) == NULL) {
+ /* misbehaving or malicious device, report? */
+ return 0;
+ }
pkt_hdr++;
continue;
}
@@ -1421,7 +1436,11 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
return 0;
}
- skb_pull(skb, (pkt_len + 7) & 0xFFF8);
+ /* XXX packet size is always (2+pkt_len), but we advance by pkt_len here? */
+ if (skb_pull(skb, (pkt_len + 7) & 0xFFF8) == NULL) {
+ /* misbehaving or malicious device, report? */
+ return 0;
+ }
pkt_hdr++;
}
return 1;
