Re: [RFC PATCH 1/3] Avoid making inappropriate requests of NETIF_F_V[46]_CSUM devices

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1454 bytes --]

On Mon, 2015-09-21 at 17:29 +0100, David Woodhouse wrote:
> 
> Did we ever resolve this? AFAICT from inspecting the code the
> virtio_net device still advertises hardware csum capabilities to the
> guest. And accepts packets which need checksumming, calling
> skb_partial_csum_set() as appropriate. Likewise tun, xen, macvtap and
> af_packet.

Here's a test case which provokes the network stack into handing a
CHECKSUM_PARTIAL skb to a device which it knows can't handle it. (It
obviously needs the AF_PACKET endianness ABI fix I sent earlier.)

You might well be right to refer to this as 'injecting unchecked crap',
but we are *gaining* injection points with the ability to do this, and
for not entirely insane reasons — people want to be able to make full
use of hardware offload capabilities.

And we *have* a safety check, to avoid handing CHECKSUM_PARTIAL buffers
to devices which can't handle them. We already do check the
capabilities of the device we end up routing it to, and complete the
checksum in software if the device can't cope.

All we're talking about here is a corner case when that existing check
doesn't actually give the right results, because it assumes a device
with NETIF_F_IP_CSUM can checksum *all* Legacy IP packets, not just TCP
and UDP.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


[-- Attachment #1.2: raw.c --]
[-- Type: text/x-csrc, Size: 3051 bytes --]



#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <sys/socket.h>
#include <netpacket/packet.h>
#include <net/ethernet.h>
#include <net/if.h>
#include <sys/ioctl.h>
#include <netinet/in.h>
#include <unistd.h>
//#include <linux/if_packet.h>
#define PACKET_VNET_HDR 15

struct virtio_net_hdr
{
    uint8_t flags;
    uint8_t gso_type;
    uint16_t hdr_len;
    uint16_t gso_size;
    uint16_t csum_start;
    uint16_t csum_offset;
};
#define VIRTIO_NET_HDR_F_NEEDS_CSUM     1

unsigned char eth_hdr[] = {
	0x52, 0x54, 0x00, 0x3a, 0xbe, 0x28, 0x52, 0x54, 0x00, 0x74, 0x2f, 0xfd, 0x08, 0x00
};
unsigned char icmp_pkt[] = {
        0x45, 0x00, 0x00, 0x54, 0x11, 0xec, 0x40, 0x00, 0x40, 0x01, 0xb3, 0x58, 0xc0, 0xa8, 0x7a, 0x12,
        0xc0, 0xa8, 0x7a, 0x01, 0x08, 0x00, 0x00, 0x00, 0x07, 0xd2, 0x00, 0x01, 0x63, 0x7f, 0x02, 0x56,
        0x00, 0x00, 0x00, 0x00, 0x7c, 0x1b, 0x0b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x11, 0x12, 0x13,
        0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23,
        0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x32, 0x33,
        0x34, 0x35, 0x36, 0x37
};

unsigned char udp_pkt[] = {
	0x45, 0x00, 0x00, 0x22, 0xc4, 0x25, 0x40, 0x00, 0x40, 0x11, 0x01, 0x41, 0xc0, 0xa8, 0x7a, 0x12,
        0xc0, 0xa8, 0x7a, 0x01, 0xae, 0xc7, 0x1f, 0x90, 0x00, 0x0e, 0x75, 0x84, 0x68, 0x65, 0x6c, 0x6c,
        0x6f, 0x0a
};

int main(int argc, char **argv)
{
	struct ifreq req;
	struct sockaddr_ll lladdr;
	int fd, ret, val;
	struct {
		struct virtio_net_hdr vnet;
		unsigned char eth[14];
		unsigned char pkt[2048];
	} buf;

	if (argc != 2) {
		fprintf(stderr, "Usage: %s <ifname>\n", argv[0]);
		exit(1);
	}
	fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_IP));
	if (fd < 0) {
		perror("socket");
		exit(1);
	}

	memset(&req, 0, sizeof(req));
	strncpy(req.ifr_name, argv[1], IFNAMSIZ-1);
	ret = ioctl(fd, SIOCGIFINDEX, &req);
	if (ret < 0) {
		perror("SIOGIFINDEX");
		exit(1);
	}

	memset(&lladdr, 0, sizeof(lladdr));
	lladdr.sll_family   = AF_PACKET;
	lladdr.sll_protocol = htons(ETH_P_IP);
	lladdr.sll_ifindex  = req.ifr_ifindex;
	ret = bind(fd, (const struct sockaddr *)&lladdr, sizeof(lladdr));
	if (ret < 0) {
		perror("bind");
		exit(1);
	}

        val = 1;
        ret = setsockopt(fd, SOL_PACKET, PACKET_VNET_HDR, (const char *)&val,
			 sizeof(val));
        if (ret < 0) {
		perror("setsockopt(SOL_PACKET, PACKET_VNET_HDR)");
		exit(1);
	}

	memset(&buf.vnet, 0, sizeof(buf.vnet));
	memcpy(&buf.eth, eth_hdr, sizeof(eth_hdr));

	memcpy(&buf.pkt, udp_pkt, sizeof(udp_pkt));
	buf.vnet.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
	buf.vnet.csum_start = 0x22;
	buf.vnet.csum_offset = 0x6;
	if (write(fd, (void *)&buf, sizeof(buf.vnet) + 14 + sizeof(udp_pkt)) < 0) {
	        perror("Write UDP packet");
		exit(1);
	}

	memcpy(&buf.pkt, icmp_pkt, sizeof(icmp_pkt));
	buf.vnet.csum_offset = 0x2;
	if (write(fd, (void *)&buf, sizeof(buf.vnet) + 14 + sizeof(icmp_pkt)) < 0) {
		perror("Write ICMP packet");
		exit(1);
	}
	return 0;
}

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]