From: David Vrabel <david.vrabel@citrix•com>
To: Ian Campbell <Ian.Campbell@citrix•com>
Cc: Wei Liu <wei.liu2@citrix•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"xen-devel@lists•xen.org" <xen-devel@lists•xen.org>,
"annie.li@oracle•com" <annie.li@oracle•com>,
"konrad.wilk@oracle•com" <konrad.wilk@oracle•com>
Subject: Re: [Xen-devel] [PATCH 2/4] xen-netfront: drop skb when skb->len > 65535
Date: Mon, 18 Mar 2013 14:00:00 +0000 [thread overview]
Message-ID: <51471DE0.9060506@citrix.com> (raw)
In-Reply-To: <1363614500.30193.47.camel@zakaz.uk.xensource.com>
On 18/03/13 13:48, Ian Campbell wrote:
> On Mon, 2013-03-18 at 13:46 +0000, David Vrabel wrote:
>> On 18/03/13 10:35, Wei Liu wrote:
>>> The `size' field of Xen network wire format is uint16_t, anything bigger than
>>> 65535 will cause overflow.
>>
>> The backend needs to be able to handle these bad packets without
>> disconnecting the VIF -- we can't fix all the frontend drivers.
>
> Agreed, although that doesn't imply that we shouldn't fix the frontend
> where we can -- such as upstream as Wei does here.
Yes, frontends should be fixed where possible.
This is what I came up with for the backend. I don't have time to look
into it further but, Wei, feel free to use it as a starting point.
David
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index cd49ba9..18e2671 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -899,10 +899,11 @@ static void netbk_fatal_tx_err(struct xenvif *vif)
static int netbk_count_requests(struct xenvif *vif,
struct xen_netif_tx_request *first,
struct xen_netif_tx_request *txp,
- int work_to_do)
+ int work_to_do, int idx)
{
RING_IDX cons = vif->tx.req_cons;
int frags = 0;
+ bool drop = false;
if (!(first->flags & XEN_NETTXF_more_data))
return 0;
@@ -922,10 +923,20 @@ static int netbk_count_requests(struct xenvif *vif,
memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + frags),
sizeof(*txp));
- if (txp->size > first->size) {
- netdev_err(vif->dev, "Frag is bigger than frame.\n");
- netbk_fatal_tx_err(vif);
- return -EIO;
+
+ /*
+ * If the guest submitted a frame >= 64 KiB then
+ * first->size overflowed and following frags will
+ * appear to be larger than the frame.
+ *
+ * This cannot be a fatal error as there are buggy
+ * frontends that do this.
+ *
+ * Consume all the frags and drop the packet.
+ */
+ if (!drop && txp->size > first->size) {
+ netdev_dbg(vif->dev, "Frag is bigger than frame.\n");
+ drop = true;
}
first->size -= txp->size;
@@ -938,6 +949,12 @@ static int netbk_count_requests(struct xenvif *vif,
return -EINVAL;
}
} while ((txp++)->flags & XEN_NETTXF_more_data);
+
+ if (drop) {
+ netbk_tx_err(vif, txp, idx + frags);
+ return -EIO;
+ }
+
return frags;
}
@@ -1327,7 +1344,7 @@ static unsigned xen_netbk_tx_build_gops(struct
xen_netbk *netbk)
continue;
}
- ret = netbk_count_requests(vif, &txreq, txfrags, work_to_do);
+ ret = netbk_count_requests(vif, &txreq, txfrags, work_to_do, idx);
if (unlikely(ret < 0))
continue;
next prev parent reply other threads:[~2013-03-18 14:00 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-18 10:35 [PATCH 0/4] Bundle fixes for xen-netfront/back Wei Liu
2013-03-18 10:35 ` [PATCH 1/4] xen-netfront: remove unused variable `extra' Wei Liu
2013-03-18 11:42 ` Ian Campbell
2013-03-18 12:04 ` Wei Liu
2013-03-18 12:14 ` Ian Campbell
2013-03-19 2:39 ` annie li
2013-03-19 3:02 ` [Xen-devel] " James Harper
2013-03-19 9:28 ` Paul Durrant
2013-03-19 9:53 ` annie li
2013-03-19 10:03 ` Paul Durrant
2013-03-19 15:26 ` Wei Liu
2013-04-09 14:28 ` Ian Campbell
2013-03-18 10:35 ` [PATCH 2/4] xen-netfront: drop skb when skb->len > 65535 Wei Liu
2013-03-18 11:42 ` Ian Campbell
2013-03-18 14:40 ` Wei Liu
2013-03-18 14:54 ` Ian Campbell
2013-03-18 15:04 ` Wei Liu
2013-03-18 15:07 ` Ian Campbell
2013-03-18 15:10 ` Wei Liu
2013-03-19 21:24 ` Ben Hutchings
2013-03-19 21:28 ` Ben Hutchings
2013-04-09 14:30 ` Ian Campbell
2013-04-09 14:45 ` Ben Hutchings
2013-04-09 14:53 ` [Xen-devel] " Christoph Egger
2013-04-09 14:59 ` Ben Hutchings
2013-03-18 13:44 ` Konrad Rzeszutek Wilk
2013-03-18 13:46 ` [Xen-devel] " David Vrabel
2013-03-18 13:48 ` Ian Campbell
2013-03-18 14:00 ` David Vrabel [this message]
2013-03-18 14:19 ` Wei Liu
2013-03-19 13:40 ` David Vrabel
2013-03-19 15:23 ` Wei Liu
2013-03-20 20:02 ` David Vrabel
2013-03-21 13:40 ` Wei Liu
2013-03-21 14:11 ` David Vrabel
2013-03-21 14:15 ` Wei Liu
2013-03-21 14:20 ` Wei Liu
2013-03-19 1:35 ` annie li
2013-03-19 20:13 ` Nick Pegg
2013-03-18 10:35 ` [PATCH 3/4] xen-netback: remove skb in xen_netbk_alloc_page Wei Liu
2013-03-18 11:37 ` Ian Campbell
2013-03-18 10:35 ` [PATCH 4/4] xen-netback: coalesce slots before copying Wei Liu
2013-03-18 12:07 ` Ian Campbell
2013-03-21 18:37 ` Wei Liu
2013-03-18 13:09 ` James Harper
2013-03-18 13:27 ` James Harper
2013-03-21 19:08 ` Wei Liu
2013-03-21 22:14 ` [Xen-devel] " James Harper
2013-03-22 11:06 ` Wei Liu
2013-03-22 11:19 ` James Harper
2013-03-22 11:28 ` Wei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51471DE0.9060506@citrix.com \
--to=david.vrabel@citrix$(echo .)com \
--cc=Ian.Campbell@citrix$(echo .)com \
--cc=annie.li@oracle$(echo .)com \
--cc=konrad.wilk@oracle$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=wei.liu2@citrix$(echo .)com \
--cc=xen-devel@lists$(echo .)xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox