Re: [Xen-devel] [PATCH net-next v2] xen-netfront: clean up code in xennet_release_rx_bufs

[David Vrabel <david.vrabel@citrix.com>]

On 17/01/14 15:43, annie li wrote:
> 
> No, I am trying to implement 2 patches.

I don't understand the need for two patches here, particularly when
the first patch introduces a security issue.  You can fold the following 
(untested) patch into your v2 patch and give it a try?

Thanks.

David

8<----------------------
xen-netfront: prevent unsafe reuse of rx buf pages after uninit

---
 drivers/net/xen-netfront.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 692589e..47aa599 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1134,19 +1134,32 @@ static void xennet_release_tx_bufs(struct netfront_info *np)
 
 static void xennet_release_rx_bufs(struct netfront_info *np)
 {
-	struct sk_buff *skb;
 	int id, ref;
 
 	spin_lock_bh(&np->rx_lock);
 
 	for (id = 0; id < NET_RX_RING_SIZE; id++) {
+		struct sk_buff *skb;
+		skb_frag_t *frag;
+		const struct page *page;
+
+		skb = np->rx_skbs[id];
+		if (!skb)
+			continue;
+
 		ref = np->grant_rx_ref[id];
 		if (ref == GRANT_INVALID_REF)
 			continue;
 
-		skb = np->rx_skbs[id];
-		gnttab_end_foreign_access_ref(ref, 0);
-		gnttab_release_grant_reference(&np->gref_rx_head, ref);
+		frag = &skb_shinfo(skb)->frags[0];
+		page = skb_frag_page(frag);
+
+		/* gnttab_end_foreign_access() needs a page ref until
+		 * foreign access is ended (which may be deferred).
+		 */
+		get_page(page);
+
+		gnttab_end_foreign_access(ref, 0, page);
 		np->grant_rx_ref[id] = GRANT_INVALID_REF;
 
 		kfree_skb(skb);
-- 
1.7.2.5