From: David Vrabel <david.vrabel@citrix•com>
To: David Vrabel <david.vrabel@citrix•com>
Cc: Ian Campbell <Ian.Campbell@citrix•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"annie.li@oracle•com" <annie.li@oracle•com>,
"konrad.wilk@oracle•com" <konrad.wilk@oracle•com>,
Wei Liu <wei.liu2@citrix•com>,
"xen-devel@lists•xen.org" <xen-devel@lists•xen.org>
Subject: Re: [Xen-devel] [PATCH 2/4] xen-netfront: drop skb when skb->len > 65535
Date: Wed, 20 Mar 2013 20:02:16 +0000 [thread overview]
Message-ID: <514A15C8.1050209@citrix.com> (raw)
In-Reply-To: <51471DE0.9060506@citrix.com>
On 18/03/13 14:00, David Vrabel wrote:
> 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.
Got some time to test this (or more correctly, something similar with
XCP's kernel) and some fixes to the suggested patch are needed. See below.
> 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)
idx should be of RING_IDX type.
> {
> 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);
This needs to be netbk_tx_err(vif, first, idx + frags) or the guest will
crash as we push a bunch of invalid responses.
David
> + 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-20 20:02 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
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 [this message]
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=514A15C8.1050209@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