* [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches
@ 2015-08-03 12:08 Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 1/7] rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165) Stefan Hajnoczi
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi
The following changes since commit cb48f67ad8c7b33c617d4f8144a27706e69fd688:
bsd-user: Fix operand to cpu_x86_exec (2015-07-30 12:38:49 +0100)
are available in the git repository at:
git://github.com/stefanha/qemu.git tags/rtl8139-cplus-tx-input-validation-pull-request
for you to fetch changes up to 8357946b15f0a31f73dd691b7da95f29318ed310:
rtl8139: check TCP Data Offset field (CVE-2015-5165) (2015-08-03 13:08:10 +0100)
----------------------------------------------------------------
Pull request
----------------------------------------------------------------
Stefan Hajnoczi (7):
rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165)
rtl8139: drop tautologous if (ip) {...} statement (CVE-2015-5165)
rtl8139: skip offload on short Ethernet/IP header (CVE-2015-5165)
rtl8139: check IP Header Length field (CVE-2015-5165)
rtl8139: check IP Total Length field (CVE-2015-5165)
rtl8139: skip offload on short TCP header (CVE-2015-5165)
rtl8139: check TCP Data Offset field (CVE-2015-5165)
hw/net/rtl8139.c | 367 +++++++++++++++++++++++++++++--------------------------
1 file changed, 192 insertions(+), 175 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 9+ messages in thread* [Qemu-devel] [PULL for-2.4 1/7] rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 2/7] rtl8139: drop tautologous if (ip) {...} statement (CVE-2015-5165) Stefan Hajnoczi ` (6 subsequent siblings) 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi Transmit offload needs to parse packet headers. If header fields have unexpected values the offload processing is skipped. The code currently uses nested ifs because there is relatively little input validation. The next patches will add missing input validation and a goto label is more appropriate to avoid deep if statement nesting. Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index e0db472..8731a30 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2160,28 +2160,30 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) size_t eth_payload_len = 0; int proto = be16_to_cpu(*(uint16_t *)(saved_buffer + 12)); - if (proto == ETH_P_IP) + if (proto != ETH_P_IP) { - DPRINTF("+++ C+ mode has IP packet\n"); + goto skip_offload; + } + + DPRINTF("+++ C+ mode has IP packet\n"); - /* not aligned */ - eth_payload_data = saved_buffer + ETH_HLEN; - eth_payload_len = saved_size - ETH_HLEN; + /* not aligned */ + eth_payload_data = saved_buffer + ETH_HLEN; + eth_payload_len = saved_size - ETH_HLEN; - ip = (ip_header*)eth_payload_data; + ip = (ip_header*)eth_payload_data; - if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) { - DPRINTF("+++ C+ mode packet has bad IP version %d " - "expected %d\n", IP_HEADER_VERSION(ip), - IP_HEADER_VERSION_4); - ip = NULL; - } else { - hlen = IP_HEADER_LENGTH(ip); - ip_protocol = ip->ip_p; - ip_data_len = be16_to_cpu(ip->ip_len) - hlen; - } + if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) { + DPRINTF("+++ C+ mode packet has bad IP version %d " + "expected %d\n", IP_HEADER_VERSION(ip), + IP_HEADER_VERSION_4); + goto skip_offload; } + hlen = IP_HEADER_LENGTH(ip); + ip_protocol = ip->ip_p; + ip_data_len = be16_to_cpu(ip->ip_len) - hlen; + if (ip) { if (txdw0 & CP_TX_IPCS) @@ -2377,6 +2379,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) } } +skip_offload: /* update tally counter */ ++s->tally_counters.TxOk; -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL for-2.4 2/7] rtl8139: drop tautologous if (ip) {...} statement (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 1/7] rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165) Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 3/7] rtl8139: skip offload on short Ethernet/IP header (CVE-2015-5165) Stefan Hajnoczi ` (5 subsequent siblings) 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi The previous patch stopped using the ip pointer as an indicator that the IP header is present. When we reach the if (ip) {...} statement we know ip is always non-NULL. Remove the if statement to reduce nesting. Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 309 +++++++++++++++++++++++++++---------------------------- 1 file changed, 153 insertions(+), 156 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 8731a30..db36b65 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2184,198 +2184,195 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) ip_protocol = ip->ip_p; ip_data_len = be16_to_cpu(ip->ip_len) - hlen; - if (ip) + if (txdw0 & CP_TX_IPCS) { - if (txdw0 & CP_TX_IPCS) - { - DPRINTF("+++ C+ mode need IP checksum\n"); + DPRINTF("+++ C+ mode need IP checksum\n"); - if (hlen<sizeof(ip_header) || hlen>eth_payload_len) {/* min header length */ - /* bad packet header len */ - /* or packet too short */ - } - else - { - ip->ip_sum = 0; - ip->ip_sum = ip_checksum(ip, hlen); - DPRINTF("+++ C+ mode IP header len=%d checksum=%04x\n", - hlen, ip->ip_sum); - } + if (hlen<sizeof(ip_header) || hlen>eth_payload_len) {/* min header length */ + /* bad packet header len */ + /* or packet too short */ } - - if ((txdw0 & CP_TX_LGSEN) && ip_protocol == IP_PROTO_TCP) + else { - int large_send_mss = (txdw0 >> 16) & CP_TC_LGSEN_MSS_MASK; + ip->ip_sum = 0; + ip->ip_sum = ip_checksum(ip, hlen); + DPRINTF("+++ C+ mode IP header len=%d checksum=%04x\n", + hlen, ip->ip_sum); + } + } - DPRINTF("+++ C+ mode offloaded task TSO MTU=%d IP data %d " - "frame data %d specified MSS=%d\n", ETH_MTU, - ip_data_len, saved_size - ETH_HLEN, large_send_mss); + if ((txdw0 & CP_TX_LGSEN) && ip_protocol == IP_PROTO_TCP) + { + int large_send_mss = (txdw0 >> 16) & CP_TC_LGSEN_MSS_MASK; - int tcp_send_offset = 0; - int send_count = 0; + DPRINTF("+++ C+ mode offloaded task TSO MTU=%d IP data %d " + "frame data %d specified MSS=%d\n", ETH_MTU, + ip_data_len, saved_size - ETH_HLEN, large_send_mss); - /* maximum IP header length is 60 bytes */ - uint8_t saved_ip_header[60]; + int tcp_send_offset = 0; + int send_count = 0; - /* save IP header template; data area is used in tcp checksum calculation */ - memcpy(saved_ip_header, eth_payload_data, hlen); + /* maximum IP header length is 60 bytes */ + uint8_t saved_ip_header[60]; - /* a placeholder for checksum calculation routine in tcp case */ - uint8_t *data_to_checksum = eth_payload_data + hlen - 12; - // size_t data_to_checksum_len = eth_payload_len - hlen + 12; + /* save IP header template; data area is used in tcp checksum calculation */ + memcpy(saved_ip_header, eth_payload_data, hlen); - /* pointer to TCP header */ - tcp_header *p_tcp_hdr = (tcp_header*)(eth_payload_data + hlen); + /* a placeholder for checksum calculation routine in tcp case */ + uint8_t *data_to_checksum = eth_payload_data + hlen - 12; + // size_t data_to_checksum_len = eth_payload_len - hlen + 12; - int tcp_hlen = TCP_HEADER_DATA_OFFSET(p_tcp_hdr); + /* pointer to TCP header */ + tcp_header *p_tcp_hdr = (tcp_header*)(eth_payload_data + hlen); - /* ETH_MTU = ip header len + tcp header len + payload */ - int tcp_data_len = ip_data_len - tcp_hlen; - int tcp_chunk_size = ETH_MTU - hlen - tcp_hlen; + int tcp_hlen = TCP_HEADER_DATA_OFFSET(p_tcp_hdr); - DPRINTF("+++ C+ mode TSO IP data len %d TCP hlen %d TCP " - "data len %d TCP chunk size %d\n", ip_data_len, - tcp_hlen, tcp_data_len, tcp_chunk_size); + /* ETH_MTU = ip header len + tcp header len + payload */ + int tcp_data_len = ip_data_len - tcp_hlen; + int tcp_chunk_size = ETH_MTU - hlen - tcp_hlen; - /* note the cycle below overwrites IP header data, - but restores it from saved_ip_header before sending packet */ + DPRINTF("+++ C+ mode TSO IP data len %d TCP hlen %d TCP " + "data len %d TCP chunk size %d\n", ip_data_len, + tcp_hlen, tcp_data_len, tcp_chunk_size); - int is_last_frame = 0; + /* note the cycle below overwrites IP header data, + but restores it from saved_ip_header before sending packet */ - for (tcp_send_offset = 0; tcp_send_offset < tcp_data_len; tcp_send_offset += tcp_chunk_size) - { - uint16_t chunk_size = tcp_chunk_size; - - /* check if this is the last frame */ - if (tcp_send_offset + tcp_chunk_size >= tcp_data_len) - { - is_last_frame = 1; - chunk_size = tcp_data_len - tcp_send_offset; - } - - DPRINTF("+++ C+ mode TSO TCP seqno %08x\n", - be32_to_cpu(p_tcp_hdr->th_seq)); - - /* add 4 TCP pseudoheader fields */ - /* copy IP source and destination fields */ - memcpy(data_to_checksum, saved_ip_header + 12, 8); - - DPRINTF("+++ C+ mode TSO calculating TCP checksum for " - "packet with %d bytes data\n", tcp_hlen + - chunk_size); - - if (tcp_send_offset) - { - memcpy((uint8_t*)p_tcp_hdr + tcp_hlen, (uint8_t*)p_tcp_hdr + tcp_hlen + tcp_send_offset, chunk_size); - } - - /* keep PUSH and FIN flags only for the last frame */ - if (!is_last_frame) - { - TCP_HEADER_CLEAR_FLAGS(p_tcp_hdr, TCP_FLAG_PUSH|TCP_FLAG_FIN); - } - - /* recalculate TCP checksum */ - ip_pseudo_header *p_tcpip_hdr = (ip_pseudo_header *)data_to_checksum; - p_tcpip_hdr->zeros = 0; - p_tcpip_hdr->ip_proto = IP_PROTO_TCP; - p_tcpip_hdr->ip_payload = cpu_to_be16(tcp_hlen + chunk_size); - - p_tcp_hdr->th_sum = 0; - - int tcp_checksum = ip_checksum(data_to_checksum, tcp_hlen + chunk_size + 12); - DPRINTF("+++ C+ mode TSO TCP checksum %04x\n", - tcp_checksum); - - p_tcp_hdr->th_sum = tcp_checksum; - - /* restore IP header */ - memcpy(eth_payload_data, saved_ip_header, hlen); - - /* set IP data length and recalculate IP checksum */ - ip->ip_len = cpu_to_be16(hlen + tcp_hlen + chunk_size); - - /* increment IP id for subsequent frames */ - ip->ip_id = cpu_to_be16(tcp_send_offset/tcp_chunk_size + be16_to_cpu(ip->ip_id)); - - ip->ip_sum = 0; - ip->ip_sum = ip_checksum(eth_payload_data, hlen); - DPRINTF("+++ C+ mode TSO IP header len=%d " - "checksum=%04x\n", hlen, ip->ip_sum); - - int tso_send_size = ETH_HLEN + hlen + tcp_hlen + chunk_size; - DPRINTF("+++ C+ mode TSO transferring packet size " - "%d\n", tso_send_size); - rtl8139_transfer_frame(s, saved_buffer, tso_send_size, - 0, (uint8_t *) dot1q_buffer); - - /* add transferred count to TCP sequence number */ - p_tcp_hdr->th_seq = cpu_to_be32(chunk_size + be32_to_cpu(p_tcp_hdr->th_seq)); - ++send_count; - } + int is_last_frame = 0; - /* Stop sending this frame */ - saved_size = 0; - } - else if (txdw0 & (CP_TX_TCPCS|CP_TX_UDPCS)) + for (tcp_send_offset = 0; tcp_send_offset < tcp_data_len; tcp_send_offset += tcp_chunk_size) { - DPRINTF("+++ C+ mode need TCP or UDP checksum\n"); + uint16_t chunk_size = tcp_chunk_size; - /* maximum IP header length is 60 bytes */ - uint8_t saved_ip_header[60]; - memcpy(saved_ip_header, eth_payload_data, hlen); + /* check if this is the last frame */ + if (tcp_send_offset + tcp_chunk_size >= tcp_data_len) + { + is_last_frame = 1; + chunk_size = tcp_data_len - tcp_send_offset; + } - uint8_t *data_to_checksum = eth_payload_data + hlen - 12; - // size_t data_to_checksum_len = eth_payload_len - hlen + 12; + DPRINTF("+++ C+ mode TSO TCP seqno %08x\n", + be32_to_cpu(p_tcp_hdr->th_seq)); /* add 4 TCP pseudoheader fields */ /* copy IP source and destination fields */ memcpy(data_to_checksum, saved_ip_header + 12, 8); - if ((txdw0 & CP_TX_TCPCS) && ip_protocol == IP_PROTO_TCP) - { - DPRINTF("+++ C+ mode calculating TCP checksum for " - "packet with %d bytes data\n", ip_data_len); - - ip_pseudo_header *p_tcpip_hdr = (ip_pseudo_header *)data_to_checksum; - p_tcpip_hdr->zeros = 0; - p_tcpip_hdr->ip_proto = IP_PROTO_TCP; - p_tcpip_hdr->ip_payload = cpu_to_be16(ip_data_len); - - tcp_header* p_tcp_hdr = (tcp_header *) (data_to_checksum+12); - - p_tcp_hdr->th_sum = 0; - - int tcp_checksum = ip_checksum(data_to_checksum, ip_data_len + 12); - DPRINTF("+++ C+ mode TCP checksum %04x\n", - tcp_checksum); + DPRINTF("+++ C+ mode TSO calculating TCP checksum for " + "packet with %d bytes data\n", tcp_hlen + + chunk_size); - p_tcp_hdr->th_sum = tcp_checksum; - } - else if ((txdw0 & CP_TX_UDPCS) && ip_protocol == IP_PROTO_UDP) + if (tcp_send_offset) { - DPRINTF("+++ C+ mode calculating UDP checksum for " - "packet with %d bytes data\n", ip_data_len); + memcpy((uint8_t*)p_tcp_hdr + tcp_hlen, (uint8_t*)p_tcp_hdr + tcp_hlen + tcp_send_offset, chunk_size); + } - ip_pseudo_header *p_udpip_hdr = (ip_pseudo_header *)data_to_checksum; - p_udpip_hdr->zeros = 0; - p_udpip_hdr->ip_proto = IP_PROTO_UDP; - p_udpip_hdr->ip_payload = cpu_to_be16(ip_data_len); + /* keep PUSH and FIN flags only for the last frame */ + if (!is_last_frame) + { + TCP_HEADER_CLEAR_FLAGS(p_tcp_hdr, TCP_FLAG_PUSH|TCP_FLAG_FIN); + } - udp_header *p_udp_hdr = (udp_header *) (data_to_checksum+12); + /* recalculate TCP checksum */ + ip_pseudo_header *p_tcpip_hdr = (ip_pseudo_header *)data_to_checksum; + p_tcpip_hdr->zeros = 0; + p_tcpip_hdr->ip_proto = IP_PROTO_TCP; + p_tcpip_hdr->ip_payload = cpu_to_be16(tcp_hlen + chunk_size); - p_udp_hdr->uh_sum = 0; + p_tcp_hdr->th_sum = 0; - int udp_checksum = ip_checksum(data_to_checksum, ip_data_len + 12); - DPRINTF("+++ C+ mode UDP checksum %04x\n", - udp_checksum); + int tcp_checksum = ip_checksum(data_to_checksum, tcp_hlen + chunk_size + 12); + DPRINTF("+++ C+ mode TSO TCP checksum %04x\n", + tcp_checksum); - p_udp_hdr->uh_sum = udp_checksum; - } + p_tcp_hdr->th_sum = tcp_checksum; /* restore IP header */ memcpy(eth_payload_data, saved_ip_header, hlen); + + /* set IP data length and recalculate IP checksum */ + ip->ip_len = cpu_to_be16(hlen + tcp_hlen + chunk_size); + + /* increment IP id for subsequent frames */ + ip->ip_id = cpu_to_be16(tcp_send_offset/tcp_chunk_size + be16_to_cpu(ip->ip_id)); + + ip->ip_sum = 0; + ip->ip_sum = ip_checksum(eth_payload_data, hlen); + DPRINTF("+++ C+ mode TSO IP header len=%d " + "checksum=%04x\n", hlen, ip->ip_sum); + + int tso_send_size = ETH_HLEN + hlen + tcp_hlen + chunk_size; + DPRINTF("+++ C+ mode TSO transferring packet size " + "%d\n", tso_send_size); + rtl8139_transfer_frame(s, saved_buffer, tso_send_size, + 0, (uint8_t *) dot1q_buffer); + + /* add transferred count to TCP sequence number */ + p_tcp_hdr->th_seq = cpu_to_be32(chunk_size + be32_to_cpu(p_tcp_hdr->th_seq)); + ++send_count; } + + /* Stop sending this frame */ + saved_size = 0; + } + else if (txdw0 & (CP_TX_TCPCS|CP_TX_UDPCS)) + { + DPRINTF("+++ C+ mode need TCP or UDP checksum\n"); + + /* maximum IP header length is 60 bytes */ + uint8_t saved_ip_header[60]; + memcpy(saved_ip_header, eth_payload_data, hlen); + + uint8_t *data_to_checksum = eth_payload_data + hlen - 12; + // size_t data_to_checksum_len = eth_payload_len - hlen + 12; + + /* add 4 TCP pseudoheader fields */ + /* copy IP source and destination fields */ + memcpy(data_to_checksum, saved_ip_header + 12, 8); + + if ((txdw0 & CP_TX_TCPCS) && ip_protocol == IP_PROTO_TCP) + { + DPRINTF("+++ C+ mode calculating TCP checksum for " + "packet with %d bytes data\n", ip_data_len); + + ip_pseudo_header *p_tcpip_hdr = (ip_pseudo_header *)data_to_checksum; + p_tcpip_hdr->zeros = 0; + p_tcpip_hdr->ip_proto = IP_PROTO_TCP; + p_tcpip_hdr->ip_payload = cpu_to_be16(ip_data_len); + + tcp_header* p_tcp_hdr = (tcp_header *) (data_to_checksum+12); + + p_tcp_hdr->th_sum = 0; + + int tcp_checksum = ip_checksum(data_to_checksum, ip_data_len + 12); + DPRINTF("+++ C+ mode TCP checksum %04x\n", + tcp_checksum); + + p_tcp_hdr->th_sum = tcp_checksum; + } + else if ((txdw0 & CP_TX_UDPCS) && ip_protocol == IP_PROTO_UDP) + { + DPRINTF("+++ C+ mode calculating UDP checksum for " + "packet with %d bytes data\n", ip_data_len); + + ip_pseudo_header *p_udpip_hdr = (ip_pseudo_header *)data_to_checksum; + p_udpip_hdr->zeros = 0; + p_udpip_hdr->ip_proto = IP_PROTO_UDP; + p_udpip_hdr->ip_payload = cpu_to_be16(ip_data_len); + + udp_header *p_udp_hdr = (udp_header *) (data_to_checksum+12); + + p_udp_hdr->uh_sum = 0; + + int udp_checksum = ip_checksum(data_to_checksum, ip_data_len + 12); + DPRINTF("+++ C+ mode UDP checksum %04x\n", + udp_checksum); + + p_udp_hdr->uh_sum = udp_checksum; + } + + /* restore IP header */ + memcpy(eth_payload_data, saved_ip_header, hlen); } } -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL for-2.4 3/7] rtl8139: skip offload on short Ethernet/IP header (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 1/7] rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165) Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 2/7] rtl8139: drop tautologous if (ip) {...} statement (CVE-2015-5165) Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 4/7] rtl8139: check IP Header Length field (CVE-2015-5165) Stefan Hajnoczi ` (4 subsequent siblings) 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi Transmit offload features access Ethernet and IP headers the packet. If the packet is too short we must not attempt to access header fields: int proto = be16_to_cpu(*(uint16_t *)(saved_buffer + 12)); ... eth_payload_data = saved_buffer + ETH_HLEN; ... ip = (ip_header*)eth_payload_data; if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) { Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index db36b65..1c62076 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2150,6 +2150,11 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) { DPRINTF("+++ C+ mode offloaded task checksum\n"); + /* Large enough for Ethernet and IP headers? */ + if (saved_size < ETH_HLEN + sizeof(ip_header)) { + goto skip_offload; + } + /* ip packet header */ ip_header *ip = NULL; int hlen = 0; -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL for-2.4 4/7] rtl8139: check IP Header Length field (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi ` (2 preceding siblings ...) 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 3/7] rtl8139: skip offload on short Ethernet/IP header (CVE-2015-5165) Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 5/7] rtl8139: check IP Total " Stefan Hajnoczi ` (3 subsequent siblings) 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi The IP Header Length field was only checked in the IP checksum case, but is used in other cases too. Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 1c62076..4eaa352 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2186,6 +2186,10 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) } hlen = IP_HEADER_LENGTH(ip); + if (hlen < sizeof(ip_header) || hlen > eth_payload_len) { + goto skip_offload; + } + ip_protocol = ip->ip_p; ip_data_len = be16_to_cpu(ip->ip_len) - hlen; @@ -2193,17 +2197,10 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) { DPRINTF("+++ C+ mode need IP checksum\n"); - if (hlen<sizeof(ip_header) || hlen>eth_payload_len) {/* min header length */ - /* bad packet header len */ - /* or packet too short */ - } - else - { - ip->ip_sum = 0; - ip->ip_sum = ip_checksum(ip, hlen); - DPRINTF("+++ C+ mode IP header len=%d checksum=%04x\n", - hlen, ip->ip_sum); - } + ip->ip_sum = 0; + ip->ip_sum = ip_checksum(ip, hlen); + DPRINTF("+++ C+ mode IP header len=%d checksum=%04x\n", + hlen, ip->ip_sum); } if ((txdw0 & CP_TX_LGSEN) && ip_protocol == IP_PROTO_TCP) -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL for-2.4 5/7] rtl8139: check IP Total Length field (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi ` (3 preceding siblings ...) 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 4/7] rtl8139: check IP Header Length field (CVE-2015-5165) Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 6/7] rtl8139: skip offload on short TCP header (CVE-2015-5165) Stefan Hajnoczi ` (2 subsequent siblings) 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi The IP Total Length field includes the IP header and data. Make sure it is valid and does not exceed the Ethernet payload size. Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 4eaa352..6859d76 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2191,7 +2191,12 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) } ip_protocol = ip->ip_p; - ip_data_len = be16_to_cpu(ip->ip_len) - hlen; + + ip_data_len = be16_to_cpu(ip->ip_len); + if (ip_data_len < hlen || ip_data_len > eth_payload_len) { + goto skip_offload; + } + ip_data_len -= hlen; if (txdw0 & CP_TX_IPCS) { -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL for-2.4 6/7] rtl8139: skip offload on short TCP header (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi ` (4 preceding siblings ...) 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 5/7] rtl8139: check IP Total " Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 7/7] rtl8139: check TCP Data Offset field (CVE-2015-5165) Stefan Hajnoczi 2015-08-03 13:08 ` [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Peter Maydell 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi TCP Large Segment Offload accesses the TCP header in the packet. If the packet is too short we must not attempt to access header fields: tcp_header *p_tcp_hdr = (tcp_header*)(eth_payload_data + hlen); int tcp_hlen = TCP_HEADER_DATA_OFFSET(p_tcp_hdr); Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 6859d76..fa01934 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2210,6 +2210,11 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) if ((txdw0 & CP_TX_LGSEN) && ip_protocol == IP_PROTO_TCP) { + /* Large enough for the TCP header? */ + if (ip_data_len < sizeof(tcp_header)) { + goto skip_offload; + } + int large_send_mss = (txdw0 >> 16) & CP_TC_LGSEN_MSS_MASK; DPRINTF("+++ C+ mode offloaded task TSO MTU=%d IP data %d " -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL for-2.4 7/7] rtl8139: check TCP Data Offset field (CVE-2015-5165) 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi ` (5 preceding siblings ...) 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 6/7] rtl8139: skip offload on short TCP header (CVE-2015-5165) Stefan Hajnoczi @ 2015-08-03 12:08 ` Stefan Hajnoczi 2015-08-03 13:08 ` [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Peter Maydell 7 siblings, 0 replies; 9+ messages in thread From: Stefan Hajnoczi @ 2015-08-03 12:08 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Maydell, Jason Wang, qemu-stable, Stefan Hajnoczi The TCP Data Offset field contains the length of the header. Make sure it is valid and does not exceed the IP data length. Reported-by: 朱东海(启路) <donghai.zdh@alibaba-inc.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index fa01934..edbb61c 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2239,6 +2239,11 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) int tcp_hlen = TCP_HEADER_DATA_OFFSET(p_tcp_hdr); + /* Invalid TCP data offset? */ + if (tcp_hlen < sizeof(tcp_header) || tcp_hlen > ip_data_len) { + goto skip_offload; + } + /* ETH_MTU = ip header len + tcp header len + payload */ int tcp_data_len = ip_data_len - tcp_hlen; int tcp_chunk_size = ETH_MTU - hlen - tcp_hlen; -- 2.4.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches 2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi ` (6 preceding siblings ...) 2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 7/7] rtl8139: check TCP Data Offset field (CVE-2015-5165) Stefan Hajnoczi @ 2015-08-03 13:08 ` Peter Maydell 7 siblings, 0 replies; 9+ messages in thread From: Peter Maydell @ 2015-08-03 13:08 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Jason Wang, QEMU Developers, qemu-stable On 3 August 2015 at 13:08, Stefan Hajnoczi <stefanha@redhat.com> wrote: > The following changes since commit cb48f67ad8c7b33c617d4f8144a27706e69fd688: > > bsd-user: Fix operand to cpu_x86_exec (2015-07-30 12:38:49 +0100) > > are available in the git repository at: > > git://github.com/stefanha/qemu.git tags/rtl8139-cplus-tx-input-validation-pull-request > > for you to fetch changes up to 8357946b15f0a31f73dd691b7da95f29318ed310: > > rtl8139: check TCP Data Offset field (CVE-2015-5165) (2015-08-03 13:08:10 +0100) > > ---------------------------------------------------------------- > Pull request > Applied, thanks. -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-08-03 13:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-03 12:08 [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 1/7] rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165) Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 2/7] rtl8139: drop tautologous if (ip) {...} statement (CVE-2015-5165) Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 3/7] rtl8139: skip offload on short Ethernet/IP header (CVE-2015-5165) Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 4/7] rtl8139: check IP Header Length field (CVE-2015-5165) Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 5/7] rtl8139: check IP Total " Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 6/7] rtl8139: skip offload on short TCP header (CVE-2015-5165) Stefan Hajnoczi
2015-08-03 12:08 ` [Qemu-devel] [PULL for-2.4 7/7] rtl8139: check TCP Data Offset field (CVE-2015-5165) Stefan Hajnoczi
2015-08-03 13:08 ` [Qemu-devel] [PULL for-2.4 0/7] Rtl8139 cplus tx input validation patches Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).