[PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[Seth Forshee]

These BUGs can be erroneously triggered by frags which refer to
tail pages within a compound page. The data in these pages may
overrun the hardware page while still being contained within the
compound page, but since compound_order() evaluates to 0 for tail
pages the assertion fails. The code already iterates through
subsequent pages correctly in this scenario, so the BUGs are
unnecessary and can be removed.

Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
Cc: <stable@vger.kernel.org> # 3.7+
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/net/xen-netfront.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index cca871346a0f..ece8d1804d13 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -496,9 +496,6 @@ static void xennet_make_frags(struct sk_buff *skb, struct netfront_queue *queue,
 		len = skb_frag_size(frag);
 		offset = frag->page_offset;
 
-		/* Data must not cross a page boundary. */
-		BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
-
 		/* Skip unused frames from start of page */
 		page += offset >> PAGE_SHIFT;
 		offset &= ~PAGE_MASK;
@@ -506,8 +503,6 @@ static void xennet_make_frags(struct sk_buff *skb, struct netfront_queue *queue,
 		while (len > 0) {
 			unsigned long bytes;
 
-			BUG_ON(offset >= PAGE_SIZE);
-
 			bytes = PAGE_SIZE - offset;
 			if (bytes > len)
 				bytes = len;
-- 
1.9.1

Re: [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[David Miller]

From: Seth Forshee <seth.forshee@canonical.com>
Date: Tue, 25 Nov 2014 20:28:24 -0600

> These BUGs can be erroneously triggered by frags which refer to
> tail pages within a compound page. The data in these pages may
> overrun the hardware page while still being contained within the
> compound page, but since compound_order() evaluates to 0 for tail
> pages the assertion fails. The code already iterates through
> subsequent pages correctly in this scenario, so the BUGs are
> unnecessary and can be removed.
> 
> Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
> Cc: <stable@vger.kernel.org> # 3.7+
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Can I get some Xen developer reviews?

Thanks.

Re: [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[Seth Forshee]

On Wed, Nov 26, 2014 at 12:28:12PM -0500, David Miller wrote:
> From: Seth Forshee <seth.forshee@canonical.com>
> Date: Tue, 25 Nov 2014 20:28:24 -0600
> 
> > These BUGs can be erroneously triggered by frags which refer to
> > tail pages within a compound page. The data in these pages may
> > overrun the hardware page while still being contained within the
> > compound page, but since compound_order() evaluates to 0 for tail
> > pages the assertion fails. The code already iterates through
> > subsequent pages correctly in this scenario, so the BUGs are
> > unnecessary and can be removed.
> > 
> > Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
> > Cc: <stable@vger.kernel.org> # 3.7+
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> Can I get some Xen developer reviews?

Fwiw this issue was discussed previously and this was the recommended
fix.

 http://article.gmane.org/gmane.linux.kernel/1825381

Since then I got some feedback from a tester that he didn't see any
problems with the BUGs removed (actually replaced with a WARN so I know
that he actually saw the condition which triggered the BUG).

Thanks,
Seth

Re: [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[David Miller]

From: Seth Forshee <seth.forshee@canonical.com>
Date: Wed, 26 Nov 2014 21:53:50 -0600

> On Wed, Nov 26, 2014 at 12:28:12PM -0500, David Miller wrote:
>> From: Seth Forshee <seth.forshee@canonical.com>
>> Date: Tue, 25 Nov 2014 20:28:24 -0600
>> 
>> > These BUGs can be erroneously triggered by frags which refer to
>> > tail pages within a compound page. The data in these pages may
>> > overrun the hardware page while still being contained within the
>> > compound page, but since compound_order() evaluates to 0 for tail
>> > pages the assertion fails. The code already iterates through
>> > subsequent pages correctly in this scenario, so the BUGs are
>> > unnecessary and can be removed.
>> > 
>> > Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
>> > Cc: <stable@vger.kernel.org> # 3.7+
>> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> 
>> Can I get some Xen developer reviews?
> 
> Fwiw this issue was discussed previously and this was the recommended
> fix.
> 
>  http://article.gmane.org/gmane.linux.kernel/1825381
> 
> Since then I got some feedback from a tester that he didn't see any
> problems with the BUGs removed (actually replaced with a WARN so I know
> that he actually saw the condition which triggered the BUG).

That's fine, but I still want a xen-netfront developer to review and
ACK this.

Re: [Xen-devel] [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[David Vrabel]

On 26/11/14 17:28, David Miller wrote:
> From: Seth Forshee <seth.forshee@canonical.com>
> Date: Tue, 25 Nov 2014 20:28:24 -0600
> 
>> These BUGs can be erroneously triggered by frags which refer to
>> tail pages within a compound page. The data in these pages may
>> overrun the hardware page while still being contained within the
>> compound page, but since compound_order() evaluates to 0 for tail
>> pages the assertion fails. The code already iterates through
>> subsequent pages correctly in this scenario, so the BUGs are
>> unnecessary and can be removed.
>>
>> Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
>> Cc: <stable@vger.kernel.org> # 3.7+
>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> Can I get some Xen developer reviews?

Sorry for the delay, I've been on holiday.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

Thanks.

David

Re: [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary

[David Miller]

From: Seth Forshee <seth.forshee@canonical.com>
Date: Tue, 25 Nov 2014 20:28:24 -0600

> These BUGs can be erroneously triggered by frags which refer to
> tail pages within a compound page. The data in these pages may
> overrun the hardware page while still being contained within the
> compound page, but since compound_order() evaluates to 0 for tail
> pages the assertion fails. The code already iterates through
> subsequent pages correctly in this scenario, so the BUGs are
> unnecessary and can be removed.
> 
> Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
> Cc: <stable@vger.kernel.org> # 3.7+
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Applied, thanks.