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

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: <52D96D73.1030803@citrix.com>
Date:	Fri, 17 Jan 2014 17:50:43 +0000
From:	David Vrabel <david.vrabel@...rix.com>
To:	annie li <annie.li@...cle.com>
CC:	Wei Liu <wei.liu2@...rix.com>, <ian.campbell@...rix.com>,
	<netdev@...r.kernel.org>, <xen-devel@...ts.xen.org>,
	<andrew.bennieston@...rix.com>, <davem@...emloft.net>
Subject: Re: [Xen-devel] [PATCH net-next v2] xen-netfront: clean up code in
 xennet_release_rx_bufs

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html