All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zoltan Kiss <zoltan.kiss@linaro.org>
To: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	David Vrabel <david.vrabel@citrix.com>,
	Stefan Bader <stefan.bader@canonical.com>,
	Jay Vosburgh <jay.vosburgh@canonical.com>,
	linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
Date: Fri, 07 Nov 2014 09:25:39 +0000	[thread overview]
Message-ID: <545C9013.1090406@linaro.org> (raw)
In-Reply-To: <20141106214940.GD44162@ubuntu-hedt>

Hi,

AFAIK in this scenario your skb frag is wrong. The page pointer should 
point to the original compound page (not a member of it), and offset 
should be set accordingly.
For example, if your compound page is 16K (4 page), then the page 
pointer should point to the first page, and if the data starts at the 
3rd page, then offset should be >8K

Zoli

On 06/11/14 21:49, Seth Forshee wrote:
> We've had several reports of hitting the following BUG_ON in
> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
> results of testing with 3.17):
>
>          /* Grant backend access to each skb fragment page. */
>          for (i = 0; i < frags; i++) {
>                  skb_frag_t *frag = skb_shinfo(skb)->frags + i;
>                  struct page *page = skb_frag_page(frag);
>
>                  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));
>
> When this happens the page in question is a "middle" page in a compound
> page (i.e. it's a tail page but not the last tail page), and the data is
> fully contained within the compound page. The data does however cross
> the hardware page boundary, and since compound_order evaluates to 0 for
> tail pages the check fails.
>
> In going over this I've been unable to determine whether the BUG_ON in
> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
> find that it's documented anywhere, and the networking code itself is a
> bit ambiguous when it comes to compound pages. On the one hand
> __skb_fill_page_desc specifically handles adding tail pages as paged
> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
> fail with data that extends into another page.
>
> Can anyone explain what the rules are here? My best guess based on
> skb_copy_bits is that paged data should never cross the hardware page
> boundary, but I'm not really sure how all of this works out when dealing
> with compound pages.
>
> Thanks,
> Seth
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

  reply	other threads:[~2014-11-07  9:25 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-06 21:49 BUG in xennet_make_frags with paged skb data Seth Forshee
2014-11-07  9:25 ` Zoltan Kiss [this message]
2014-11-07 11:22   ` [Xen-devel] " Eric Dumazet
2014-11-07 12:15     ` Stefan Bader
2014-11-07 12:15     ` [Xen-devel] " Stefan Bader
2014-11-07 12:21       ` Zoltan Kiss
2014-11-07 12:28         ` Stefan Bader
2014-11-07 12:28         ` [Xen-devel] " Stefan Bader
2014-11-07 12:21       ` Zoltan Kiss
2014-11-07 11:22   ` Eric Dumazet
2014-11-07  9:25 ` Zoltan Kiss
2014-11-07 10:44 ` David Vrabel
2014-11-07 10:44 ` David Vrabel
2014-11-07 11:08   ` Stefan Bader
2014-11-07 14:51     ` Zoltan Kiss
2014-11-07 14:51     ` [Xen-devel] " Zoltan Kiss
2014-11-07 11:08   ` Stefan Bader
2014-11-10 14:35   ` Seth Forshee
2014-11-10 14:41     ` David Vrabel
2014-11-10 16:39       ` Zoltan Kiss
2014-11-10 16:39       ` [Xen-devel] " Zoltan Kiss
2014-11-10 16:42         ` David Vrabel
2014-11-10 16:42         ` [Xen-devel] " David Vrabel
2014-11-10 17:02           ` Eric Dumazet
2014-11-10 17:02           ` Eric Dumazet
2014-11-10 17:20           ` Frediano Ziglio
2014-11-10 14:41     ` David Vrabel
2014-11-10 14:35   ` Seth Forshee

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=545C9013.1090406@linaro.org \
    --to=zoltan.kiss@linaro.org \
    --cc=boris.ostrovsky@oracle.com \
    --cc=davem@davemloft.net \
    --cc=david.vrabel@citrix.com \
    --cc=jay.vosburgh@canonical.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stefan.bader@canonical.com \
    --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: link
Be 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.