| Message ID | 1416968904-70874-1-git-send-email-seth.forshee@canonical.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)