The following patches
1/3 firewire: net: guard against rx buffer overflows
2/3 firewire: net: fix fragmented datagram_size off-by-one
3/3 firewire: net: max MTU off by one
fix a few long-standing bugs of the IP-over-1394 driver firewire-net
related to reception and transmission of fragmented datagrams:
- RX: Missing validation of fragment offset and size makes the driver
vulnerable to buffer overflows, potentially leading to remote¹ code
execution. Reported by Eyal Itkin.
¹) The vulnerability cannot be triggered by malformed IP datagrams,
but by malformed IEEE 1394 packets sent from other FireWire nodes to
the 1394 broadcast channel or to firewire-net's unicast FIFO, or can
be sent from the local node to the unicast FIFO by sufficiently
privileged userland. I.e. an attack can only originate from somewhere
on the FireWire bus, not from another network that is bridged to the
FireWire bus.
- RX: Missing validation of unfragmented and fragmented datagrams for
minimum packet size before looking at GASP header and encapsulation
header.
- RX and TX: The datagram_size field of fragmented datagrams was read
and written incorrectly; an offset of +/-1 needs to be applied. This
prevents fragmented traffic from/to nodes which run OS X, Windows XP,
or Linux' older eth1394 driver. (Traffic from Win XP would eventually
be retried with smaller MTU and possibly succeed slowly despite the
bug.)
Patch 1/3 is obviously urgent.
Patch 2/3 is a bit of a bother because while it fixes fragmented RX/TX with
OS X, Win XP, and eth1394, it does disrupt fragmented RX/TX with Linux
nodes which run an unfixed firewire-net.
Patch 3/3 will only apply in conjunction with changes that are queued up
in the net-next git tree, hence this patch will wait until net-next was
merged.
Patches 1+2/3 are already pushed out to linux1394.git "testing" and
"for-next" branches, but I still like to get review comments before I
send a pull request.
--
Stefan Richter
-======----- =-== ---=-
http://arcgraph.de/sr/
Date: Sat, 29 Oct 2016 21:28:18 +0200
The IP-over-1394 driver firewire-net lacked input validation when
handling incoming fragmented datagrams. A maliciously formed fragment
with a respectively large datagram_offset would cause a memcpy past the
datagram buffer.
So, drop any packets carrying a fragment with offset + length larger
than datagram_size.
In addition, ensure that
- GASP header, unfragmented encapsulation header, or fragment
encapsulation header actually exists before we access it,
- the encapsulated datagram or fragment is of nonzero size.
Reported-by: Eyal Itkin <[email protected]>
Cc: [email protected]
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/net.c | 51 ++++++++++++++++++++++++++++-------------
1 file changed, 35 insertions(+), 16 deletions(-)
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -578,6 +578,9 @@ static int fwnet_incoming_packet(struct
int retval;
u16 ether_type;
+ if (len <= RFC2374_UNFRAG_HDR_SIZE)
+ return 0;
+
hdr.w0 = be32_to_cpu(buf[0]);
lf = fwnet_get_hdr_lf(&hdr);
if (lf == RFC2374_HDR_UNFRAG) {
@@ -602,7 +605,12 @@ static int fwnet_incoming_packet(struct
return fwnet_finish_incoming_packet(net, skb, source_node_id,
is_broadcast, ether_type);
}
+
/* A datagram fragment has been received, now the fun begins. */
+
+ if (len <= RFC2374_FRAG_HDR_SIZE)
+ return 0;
+
hdr.w1 = ntohl(buf[1]);
buf += 2;
len -= RFC2374_FRAG_HDR_SIZE;
@@ -616,6 +624,9 @@ static int fwnet_incoming_packet(struct
datagram_label = fwnet_get_hdr_dgl(&hdr);
dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */
+ if (fg_off + len > dg_size)
+ return 0;
+
spin_lock_irqsave(&dev->lock, flags);
peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation);
@@ -722,6 +733,22 @@ static void fwnet_receive_packet(struct
fw_send_response(card, r, rcode);
}
+static int gasp_source_id(__be32 *p)
+{
+ return be32_to_cpu(p[0]) >> 16;
+}
+
+static u32 gasp_specifier_id(__be32 *p)
+{
+ return (be32_to_cpu(p[0]) & 0xffff) << 8 |
+ (be32_to_cpu(p[1]) & 0xff000000) >> 24;
+}
+
+static u32 gasp_version(__be32 *p)
+{
+ return be32_to_cpu(p[1]) & 0xffffff;
+}
+
static void fwnet_receive_broadcast(struct fw_iso_context *context,
u32 cycle, size_t header_length, void *header, void *data)
{
@@ -731,9 +758,6 @@ static void fwnet_receive_broadcast(stru
__be32 *buf_ptr;
int retval;
u32 length;
- u16 source_node_id;
- u32 specifier_id;
- u32 ver;
unsigned long offset;
unsigned long flags;
@@ -750,22 +774,17 @@ static void fwnet_receive_broadcast(stru
spin_unlock_irqrestore(&dev->lock, flags);
- specifier_id = (be32_to_cpu(buf_ptr[0]) & 0xffff) << 8
- | (be32_to_cpu(buf_ptr[1]) & 0xff000000) >> 24;
- ver = be32_to_cpu(buf_ptr[1]) & 0xffffff;
- source_node_id = be32_to_cpu(buf_ptr[0]) >> 16;
-
- if (specifier_id == IANA_SPECIFIER_ID &&
- (ver == RFC2734_SW_VERSION
+ if (length > IEEE1394_GASP_HDR_SIZE &&
+ gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID &&
+ (gasp_version(buf_ptr) == RFC2734_SW_VERSION
#if IS_ENABLED(CONFIG_IPV6)
- || ver == RFC3146_SW_VERSION
+ || gasp_version(buf_ptr) == RFC3146_SW_VERSION
#endif
- )) {
- buf_ptr += 2;
- length -= IEEE1394_GASP_HDR_SIZE;
- fwnet_incoming_packet(dev, buf_ptr, length, source_node_id,
+ ))
+ fwnet_incoming_packet(dev, buf_ptr + 2,
+ length - IEEE1394_GASP_HDR_SIZE,
+ gasp_source_id(buf_ptr),
context->card->generation, true);
- }
packet.payload_length = dev->rcv_buffer_size;
packet.interrupt = 1;
--
Stefan Richter
-======----- =-== ---=-
http://arcgraph.de/sr/
Date: Sun, 30 Oct 2016 17:32:01 +0100
RFC 2734 defines the datagram_size field in fragment encapsulation
headers thus:
datagram_size: The encoded size of the entire IP datagram. The
value of datagram_size [...] SHALL be one less than the value of
Total Length in the datagram's IP header (see STD 5, RFC 791).
Accordingly, the eth1394 driver of Linux 2.6.36 and older set and got
this field with a -/+1 offset:
ether1394_tx() /* transmit */
ether1394_encapsulate_prep()
hdr->ff.dg_size = dg_size - 1;
ether1394_data_handler() /* receive */
if (hdr->common.lf == ETH1394_HDR_LF_FF)
dg_size = hdr->ff.dg_size + 1;
else
dg_size = hdr->sf.dg_size + 1;
Likewise, I observe OS X 10.4 and Windows XP Pro SP3 to transmit 1500
byte sized datagrams in fragments with datagram_size=1499 if link
fragmentation is required.
Only firewire-net sets and gets datagram_size without this offset. The
result is lacking interoperability of firewire-net with OS X, Windows
XP, and presumably Linux' eth1394. (I did not test with the latter.)
For example, FTP data transfers to a Linux firewire-net box with max_rec
smaller than the 1500 bytes MTU
- from OS X fail entirely,
- from Win XP start out with a bunch of fragmented datagrams which
time out, then continue with unfragmented datagrams because Win XP
temporarily reduces the MTU to 576 bytes.
So let's fix firewire-net's datagram_size accessors.
Note that firewire-net thereby loses interoperability with unpatched
firewire-net, but only if link fragmentation is employed. (This happens
with large broadcast datagrams, and with large datagrams on several
FireWire CardBus cards with smaller max_rec than equivalent PCI cards,
and it can be worked around by setting a small enough MTU.)
Cc: [email protected]
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/net.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -73,13 +73,13 @@ struct rfc2734_header {
#define fwnet_get_hdr_lf(h) (((h)->w0 & 0xc0000000) >> 30)
#define fwnet_get_hdr_ether_type(h) (((h)->w0 & 0x0000ffff))
-#define fwnet_get_hdr_dg_size(h) (((h)->w0 & 0x0fff0000) >> 16)
+#define fwnet_get_hdr_dg_size(h) ((((h)->w0 & 0x0fff0000) >> 16) + 1)
#define fwnet_get_hdr_fg_off(h) (((h)->w0 & 0x00000fff))
#define fwnet_get_hdr_dgl(h) (((h)->w1 & 0xffff0000) >> 16)
-#define fwnet_set_hdr_lf(lf) ((lf) << 30)
+#define fwnet_set_hdr_lf(lf) ((lf) << 30)
#define fwnet_set_hdr_ether_type(et) (et)
-#define fwnet_set_hdr_dg_size(dgs) ((dgs) << 16)
+#define fwnet_set_hdr_dg_size(dgs) (((dgs) - 1) << 16)
#define fwnet_set_hdr_fg_off(fgo) (fgo)
#define fwnet_set_hdr_dgl(dgl) ((dgl) << 16)
@@ -622,7 +622,7 @@ static int fwnet_incoming_packet(struct
fg_off = fwnet_get_hdr_fg_off(&hdr);
}
datagram_label = fwnet_get_hdr_dgl(&hdr);
- dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */
+ dg_size = fwnet_get_hdr_dg_size(&hdr);
if (fg_off + len > dg_size)
return 0;
--
Stefan Richter
-======----- =-== ---=-
http://arcgraph.de/sr/
The latest max_mtu patch missed that datagram_size is actually one less
than the datagram's Total Length.
Fixes: 357f4aae859b ("firewire: net: really fix maximum possible MTU")
Signed-off-by: Stefan Richter <[email protected]>
---
To be applied after net-next was merged.
drivers/firewire/net.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 5d3640264f2d..655c259e37fd 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1482,9 +1482,14 @@ static int fwnet_probe(struct fw_unit *unit,
goto out;
dev->local_fifo = dev->handler.offset;
+ /*
+ * default MTU: RFC 2734 cl. 4, RFC 3146 cl. 4
+ * maximum MTU: RFC 2734 cl. 4.2, fragment encapsulation header's
+ * maximum possible datagram_size + 1 = 0xfff + 1
+ */
net->mtu = 1500U;
net->min_mtu = ETH_MIN_MTU;
- net->max_mtu = 0xfff;
+ net->max_mtu = 4096U;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-======----- =-== ---=-
http://arcgraph.de/sr/