From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com> To: xen-devel@lists.xenproject.org Cc: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>, stable@vger.kernel.org, "Boris Ostrovsky" <boris.ostrovsky@oracle.com>, "Juergen Gross" <jgross@suse.com>, netdev@vger.kernel.org (open list:NETWORKING DRIVERS), linux-kernel@vger.kernel.org (open list) Subject: [PATCH 2/6] xen-netfront: copy response out of shared buffer before accessing it Date: Mon, 30 Apr 2018 23:01:46 +0200 [thread overview] Message-ID: <98a855dceb47dbebd9c87e024084f14a5cb127f7.1525122026.git-series.marmarek@invisiblethingslab.com> (raw) In-Reply-To: <cover.7ee732ab822b728ec486a3118ec12e9c06f0f325.1525122026.git-series.marmarek@invisiblethingslab.com> In-Reply-To: <cover.7ee732ab822b728ec486a3118ec12e9c06f0f325.1525122026.git-series.marmarek@invisiblethingslab.com> Make local copy of the response, otherwise backend might modify it while frontend is already processing it - leading to time of check / time of use issue. This is complementary to XSA155. Cc: stable@vger.kernel.org Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- drivers/net/xen-netfront.c | 51 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 4dd0668..dc99763 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -387,13 +387,13 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue) rmb(); /* Ensure we see responses up to 'rp'. */ for (cons = queue->tx.rsp_cons; cons != prod; cons++) { - struct xen_netif_tx_response *txrsp; + struct xen_netif_tx_response txrsp; - txrsp = RING_GET_RESPONSE(&queue->tx, cons); - if (txrsp->status == XEN_NETIF_RSP_NULL) + RING_COPY_RESPONSE(&queue->tx, cons, &txrsp); + if (txrsp.status == XEN_NETIF_RSP_NULL) continue; - id = txrsp->id; + id = txrsp.id; skb = queue->tx_skbs[id].skb; if (unlikely(gnttab_query_foreign_access( queue->grant_tx_ref[id]) != 0)) { @@ -741,7 +741,7 @@ static int xennet_get_extras(struct netfront_queue *queue, RING_IDX rp) { - struct xen_netif_extra_info *extra; + struct xen_netif_extra_info extra; struct device *dev = &queue->info->netdev->dev; RING_IDX cons = queue->rx.rsp_cons; int err = 0; @@ -757,24 +757,23 @@ static int xennet_get_extras(struct netfront_queue *queue, break; } - extra = (struct xen_netif_extra_info *) - RING_GET_RESPONSE(&queue->rx, ++cons); + RING_COPY_RESPONSE(&queue->rx, ++cons, &extra); - if (unlikely(!extra->type || - extra->type >= XEN_NETIF_EXTRA_TYPE_MAX)) { + if (unlikely(!extra.type || + extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) { if (net_ratelimit()) dev_warn(dev, "Invalid extra type: %d\n", - extra->type); + extra.type); err = -EINVAL; } else { - memcpy(&extras[extra->type - 1], extra, - sizeof(*extra)); + memcpy(&extras[extra.type - 1], &extra, + sizeof(extra)); } skb = xennet_get_rx_skb(queue, cons); ref = xennet_get_rx_ref(queue, cons); xennet_move_rx_slot(queue, skb, ref); - } while (extra->flags & XEN_NETIF_EXTRA_FLAG_MORE); + } while (extra.flags & XEN_NETIF_EXTRA_FLAG_MORE); queue->rx.rsp_cons = cons; return err; @@ -784,28 +783,28 @@ static int xennet_get_responses(struct netfront_queue *queue, struct netfront_rx_info *rinfo, RING_IDX rp, struct sk_buff_head *list) { - struct xen_netif_rx_response *rx = &rinfo->rx; + struct xen_netif_rx_response rx = rinfo->rx; struct xen_netif_extra_info *extras = rinfo->extras; struct device *dev = &queue->info->netdev->dev; RING_IDX cons = queue->rx.rsp_cons; struct sk_buff *skb = xennet_get_rx_skb(queue, cons); grant_ref_t ref = xennet_get_rx_ref(queue, cons); - int max = MAX_SKB_FRAGS + (rx->status <= RX_COPY_THRESHOLD); + int max = MAX_SKB_FRAGS + (rx.status <= RX_COPY_THRESHOLD); int slots = 1; int err = 0; unsigned long ret; - if (rx->flags & XEN_NETRXF_extra_info) { + if (rx.flags & XEN_NETRXF_extra_info) { err = xennet_get_extras(queue, extras, rp); cons = queue->rx.rsp_cons; } for (;;) { - if (unlikely(rx->status < 0 || - rx->offset + rx->status > XEN_PAGE_SIZE)) { + if (unlikely(rx.status < 0 || + rx.offset + rx.status > XEN_PAGE_SIZE)) { if (net_ratelimit()) dev_warn(dev, "rx->offset: %u, size: %d\n", - rx->offset, rx->status); + rx.offset, rx.status); xennet_move_rx_slot(queue, skb, ref); err = -EINVAL; goto next; @@ -819,7 +818,7 @@ static int xennet_get_responses(struct netfront_queue *queue, if (ref == GRANT_INVALID_REF) { if (net_ratelimit()) dev_warn(dev, "Bad rx response id %d.\n", - rx->id); + rx.id); err = -EINVAL; goto next; } @@ -832,7 +831,7 @@ static int xennet_get_responses(struct netfront_queue *queue, __skb_queue_tail(list, skb); next: - if (!(rx->flags & XEN_NETRXF_more_data)) + if (!(rx.flags & XEN_NETRXF_more_data)) break; if (cons + slots == rp) { @@ -842,7 +841,7 @@ static int xennet_get_responses(struct netfront_queue *queue, break; } - rx = RING_GET_RESPONSE(&queue->rx, cons + slots); + RING_COPY_RESPONSE(&queue->rx, cons + slots, &rx); skb = xennet_get_rx_skb(queue, cons + slots); ref = xennet_get_rx_ref(queue, cons + slots); slots++; @@ -898,9 +897,9 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue, struct sk_buff *nskb; while ((nskb = __skb_dequeue(list))) { - struct xen_netif_rx_response *rx = - RING_GET_RESPONSE(&queue->rx, ++cons); + struct xen_netif_rx_response rx; skb_frag_t *nfrag = &skb_shinfo(nskb)->frags[0]; + RING_COPY_RESPONSE(&queue->rx, ++cons, &rx); if (shinfo->nr_frags == MAX_SKB_FRAGS) { unsigned int pull_to = NETFRONT_SKB_CB(skb)->pull_to; @@ -911,7 +910,7 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue, BUG_ON(shinfo->nr_frags >= MAX_SKB_FRAGS); skb_add_rx_frag(skb, shinfo->nr_frags, skb_frag_page(nfrag), - rx->offset, rx->status, PAGE_SIZE); + rx.offset, rx.status, PAGE_SIZE); skb_shinfo(nskb)->nr_frags = 0; kfree_skb(nskb); @@ -1007,7 +1006,7 @@ static int xennet_poll(struct napi_struct *napi, int budget) i = queue->rx.rsp_cons; work_done = 0; while ((i != rp) && (work_done < budget)) { - memcpy(rx, RING_GET_RESPONSE(&queue->rx, i), sizeof(*rx)); + RING_COPY_RESPONSE(&queue->rx, i, rx); memset(extras, 0, sizeof(rinfo.extras)); err = xennet_get_responses(queue, &rinfo, rp, &tmpq); -- git-series 0.9.1
WARNING: multiple messages have this Message-ID (diff)
From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com> To: xen-devel@lists.xenproject.org Cc: "Juergen Gross" <jgross@suse.com>, "open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>, "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>, stable@vger.kernel.org, "open list" <linux-kernel@vger.kernel.org>, "Boris Ostrovsky" <boris.ostrovsky@oracle.com> Subject: [PATCH 2/6] xen-netfront: copy response out of shared buffer before accessing it Date: Mon, 30 Apr 2018 23:01:46 +0200 [thread overview] Message-ID: <98a855dceb47dbebd9c87e024084f14a5cb127f7.1525122026.git-series.marmarek@invisiblethingslab.com> (raw) In-Reply-To: <cover.7ee732ab822b728ec486a3118ec12e9c06f0f325.1525122026.git-series.marmarek@invisiblethingslab.com> In-Reply-To: <cover.7ee732ab822b728ec486a3118ec12e9c06f0f325.1525122026.git-series.marmarek@invisiblethingslab.com> Make local copy of the response, otherwise backend might modify it while frontend is already processing it - leading to time of check / time of use issue. This is complementary to XSA155. Cc: stable@vger.kernel.org Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- drivers/net/xen-netfront.c | 51 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 4dd0668..dc99763 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -387,13 +387,13 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue) rmb(); /* Ensure we see responses up to 'rp'. */ for (cons = queue->tx.rsp_cons; cons != prod; cons++) { - struct xen_netif_tx_response *txrsp; + struct xen_netif_tx_response txrsp; - txrsp = RING_GET_RESPONSE(&queue->tx, cons); - if (txrsp->status == XEN_NETIF_RSP_NULL) + RING_COPY_RESPONSE(&queue->tx, cons, &txrsp); + if (txrsp.status == XEN_NETIF_RSP_NULL) continue; - id = txrsp->id; + id = txrsp.id; skb = queue->tx_skbs[id].skb; if (unlikely(gnttab_query_foreign_access( queue->grant_tx_ref[id]) != 0)) { @@ -741,7 +741,7 @@ static int xennet_get_extras(struct netfront_queue *queue, RING_IDX rp) { - struct xen_netif_extra_info *extra; + struct xen_netif_extra_info extra; struct device *dev = &queue->info->netdev->dev; RING_IDX cons = queue->rx.rsp_cons; int err = 0; @@ -757,24 +757,23 @@ static int xennet_get_extras(struct netfront_queue *queue, break; } - extra = (struct xen_netif_extra_info *) - RING_GET_RESPONSE(&queue->rx, ++cons); + RING_COPY_RESPONSE(&queue->rx, ++cons, &extra); - if (unlikely(!extra->type || - extra->type >= XEN_NETIF_EXTRA_TYPE_MAX)) { + if (unlikely(!extra.type || + extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) { if (net_ratelimit()) dev_warn(dev, "Invalid extra type: %d\n", - extra->type); + extra.type); err = -EINVAL; } else { - memcpy(&extras[extra->type - 1], extra, - sizeof(*extra)); + memcpy(&extras[extra.type - 1], &extra, + sizeof(extra)); } skb = xennet_get_rx_skb(queue, cons); ref = xennet_get_rx_ref(queue, cons); xennet_move_rx_slot(queue, skb, ref); - } while (extra->flags & XEN_NETIF_EXTRA_FLAG_MORE); + } while (extra.flags & XEN_NETIF_EXTRA_FLAG_MORE); queue->rx.rsp_cons = cons; return err; @@ -784,28 +783,28 @@ static int xennet_get_responses(struct netfront_queue *queue, struct netfront_rx_info *rinfo, RING_IDX rp, struct sk_buff_head *list) { - struct xen_netif_rx_response *rx = &rinfo->rx; + struct xen_netif_rx_response rx = rinfo->rx; struct xen_netif_extra_info *extras = rinfo->extras; struct device *dev = &queue->info->netdev->dev; RING_IDX cons = queue->rx.rsp_cons; struct sk_buff *skb = xennet_get_rx_skb(queue, cons); grant_ref_t ref = xennet_get_rx_ref(queue, cons); - int max = MAX_SKB_FRAGS + (rx->status <= RX_COPY_THRESHOLD); + int max = MAX_SKB_FRAGS + (rx.status <= RX_COPY_THRESHOLD); int slots = 1; int err = 0; unsigned long ret; - if (rx->flags & XEN_NETRXF_extra_info) { + if (rx.flags & XEN_NETRXF_extra_info) { err = xennet_get_extras(queue, extras, rp); cons = queue->rx.rsp_cons; } for (;;) { - if (unlikely(rx->status < 0 || - rx->offset + rx->status > XEN_PAGE_SIZE)) { + if (unlikely(rx.status < 0 || + rx.offset + rx.status > XEN_PAGE_SIZE)) { if (net_ratelimit()) dev_warn(dev, "rx->offset: %u, size: %d\n", - rx->offset, rx->status); + rx.offset, rx.status); xennet_move_rx_slot(queue, skb, ref); err = -EINVAL; goto next; @@ -819,7 +818,7 @@ static int xennet_get_responses(struct netfront_queue *queue, if (ref == GRANT_INVALID_REF) { if (net_ratelimit()) dev_warn(dev, "Bad rx response id %d.\n", - rx->id); + rx.id); err = -EINVAL; goto next; } @@ -832,7 +831,7 @@ static int xennet_get_responses(struct netfront_queue *queue, __skb_queue_tail(list, skb); next: - if (!(rx->flags & XEN_NETRXF_more_data)) + if (!(rx.flags & XEN_NETRXF_more_data)) break; if (cons + slots == rp) { @@ -842,7 +841,7 @@ static int xennet_get_responses(struct netfront_queue *queue, break; } - rx = RING_GET_RESPONSE(&queue->rx, cons + slots); + RING_COPY_RESPONSE(&queue->rx, cons + slots, &rx); skb = xennet_get_rx_skb(queue, cons + slots); ref = xennet_get_rx_ref(queue, cons + slots); slots++; @@ -898,9 +897,9 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue, struct sk_buff *nskb; while ((nskb = __skb_dequeue(list))) { - struct xen_netif_rx_response *rx = - RING_GET_RESPONSE(&queue->rx, ++cons); + struct xen_netif_rx_response rx; skb_frag_t *nfrag = &skb_shinfo(nskb)->frags[0]; + RING_COPY_RESPONSE(&queue->rx, ++cons, &rx); if (shinfo->nr_frags == MAX_SKB_FRAGS) { unsigned int pull_to = NETFRONT_SKB_CB(skb)->pull_to; @@ -911,7 +910,7 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue, BUG_ON(shinfo->nr_frags >= MAX_SKB_FRAGS); skb_add_rx_frag(skb, shinfo->nr_frags, skb_frag_page(nfrag), - rx->offset, rx->status, PAGE_SIZE); + rx.offset, rx.status, PAGE_SIZE); skb_shinfo(nskb)->nr_frags = 0; kfree_skb(nskb); @@ -1007,7 +1006,7 @@ static int xennet_poll(struct napi_struct *napi, int budget) i = queue->rx.rsp_cons; work_done = 0; while ((i != rp) && (work_done < budget)) { - memcpy(rx, RING_GET_RESPONSE(&queue->rx, i), sizeof(*rx)); + RING_COPY_RESPONSE(&queue->rx, i, rx); memset(extras, 0, sizeof(rinfo.extras)); err = xennet_get_responses(queue, &rinfo, rp, &tmpq); -- git-series 0.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2018-04-30 21:05 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-30 21:01 [PATCH 0/6] Fix XSA-155-like bugs in frontend drivers Marek Marczykowski-Górecki 2018-04-30 21:01 ` [PATCH 1/6] xen: Add RING_COPY_RESPONSE() Marek Marczykowski-Górecki 2018-04-30 21:25 ` Boris Ostrovsky 2018-04-30 21:25 ` Boris Ostrovsky 2018-04-30 21:27 ` Marek Marczykowski-Górecki 2018-04-30 21:41 ` Boris Ostrovsky 2018-04-30 21:41 ` Boris Ostrovsky 2018-04-30 21:27 ` Marek Marczykowski-Górecki 2018-04-30 21:01 ` Marek Marczykowski-Górecki 2018-04-30 21:01 ` Marek Marczykowski-Górecki [this message] 2018-04-30 21:01 ` [PATCH 2/6] xen-netfront: copy response out of shared buffer before accessing it Marek Marczykowski-Górecki 2018-05-02 5:20 ` Oleksandr Andrushchenko 2018-05-02 5:20 ` [Xen-devel] " Oleksandr Andrushchenko 2018-04-30 21:01 ` [PATCH 3/6] xen-netfront: do not use data already exposed to backend Marek Marczykowski-Górecki 2018-04-30 21:01 ` Marek Marczykowski-Górecki 2018-04-30 21:01 ` [PATCH 4/6] xen-netfront: add range check for Tx response id Marek Marczykowski-Górecki 2018-05-01 10:05 ` Wei Liu 2018-05-01 10:05 ` [Xen-devel] " Wei Liu 2018-05-01 10:05 ` Wei Liu 2018-04-30 21:01 ` Marek Marczykowski-Górecki 2018-04-30 21:01 ` [PATCH 5/6] xen-blkfront: make local copy of response before using it Marek Marczykowski-Górecki 2018-04-30 21:01 ` Marek Marczykowski-Górecki 2018-04-30 21:01 ` [PATCH 6/6] xen-blkfront: prepare request locally, only then put it on the shared ring Marek Marczykowski-Górecki 2018-04-30 21:01 ` Marek Marczykowski-Górecki 2018-05-01 8:22 ` Roger Pau Monné 2018-05-01 8:22 ` Roger Pau Monné 2018-05-01 8:22 ` Roger Pau Monné 2018-05-01 9:15 ` Roger Pau Monné 2018-05-01 9:15 ` [Xen-devel] " Roger Pau Monné 2018-05-01 9:15 ` Roger Pau Monné 2018-05-01 10:12 ` [PATCH 0/6] Fix XSA-155-like bugs in frontend drivers 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=98a855dceb47dbebd9c87e024084f14a5cb127f7.1525122026.git-series.marmarek@invisiblethingslab.com \ --to=marmarek@invisiblethingslab.com \ --cc=boris.ostrovsky@oracle.com \ --cc=jgross@suse.com \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=stable@vger.kernel.org \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.