netdev - Re: [PATCH 3/3] vsockmon: Add virtio vsock hooks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160810115918.GC10461@stefanha-x1.localdomain>
Date:	Wed, 10 Aug 2016 12:59:18 +0100
From:	Stefan Hajnoczi <stefanha@...hat.com>
To:	ggarcia@...a.uab.cat
Cc:	netdev@...r.kernel.org, jhansen@...are.com, mst@...hat.com
Subject: Re: [PATCH 3/3] vsockmon: Add virtio vsock hooks

On Mon, Aug 08, 2016 at 06:14:42PM +0200, ggarcia@...a.uab.cat wrote:
> +static struct sk_buff *
> +virtio_vsock_pkt_vsockmon_to_vsockmon_skb(struct virtio_vsock_pkt *pkt)
> +{
> +	struct sk_buff *skb;
> +	struct af_vsockmon_hdr *hdr;
> +	void *payload;
> +
> +	u32 skb_len = sizeof(struct af_vsockmon_hdr) + pkt->len;
> +
> +	skb = alloc_skb(skb_len, GFP_ATOMIC);
> +	if (!skb)
> +		return NULL;
> +
> +	hdr = (struct af_vsockmon_hdr *) skb_put(skb, sizeof(*hdr));
> +
> +	hdr->src_cid = pkt->hdr.src_cid;
> +	hdr->src_port = pkt->hdr.src_port;
> +	hdr->dst_cid = pkt->hdr.dst_cid;
> +	hdr->dst_port = pkt->hdr.dst_port;
> +	hdr->t = cpu_to_le16(AF_VSOCK_T_VIRTIO);
> +	hdr->len = cpu_to_le16(sizeof(hdr->t_hdr));
> +
> +	switch(pkt->hdr.op) {

Missing le16_to_cpu()

> +	case VIRTIO_VSOCK_OP_REQUEST:
> +	case VIRTIO_VSOCK_OP_RESPONSE:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_CONNECT);
> +		break;
> +	case VIRTIO_VSOCK_OP_RST:
> +	case VIRTIO_VSOCK_OP_SHUTDOWN:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_DISCONNECT);
> +		break;
> +	case VIRTIO_VSOCK_OP_RW:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_PAYLOAD);
> +		break;
> +	case VIRTIO_VSOCK_OP_CREDIT_UPDATE:
> +	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_CONTROL);
> +		break;
> +	default:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_UNKNOWN);
> +		break;
> +	}
> +
> +	hdr->t_hdr.virtio_hdr = pkt->hdr;
> +
> +	if (pkt->len) {
> +		payload = skb_put(skb, pkt->len);
> +		memcpy(payload, pkt->buf, pkt->len);
> +	}
> +
> +	return skb;
> +}
> +
> +static void vsock_deliver_tap_pkt(struct virtio_vsock_pkt *pkt)
> +{
> +	struct sk_buff *skb = virtio_vsock_pkt_to_vsockmon_skb(pkt);
> +	if (skb) {
> +		vsock_deliver_tap(skb);
> +		kfree_skb(skb);

Should this be consume_skb()?  The function's doc comment says:

 *	Functions identically to kfree_skb, but kfree_skb assumes that the frame
 *	is being dropped after a failure and notes that

This isn't a failure case so kfree_skb() is not the right function.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)