netdev - Re: [Xen-devel] Xen-unstable Linux 3.14-rc3 and 3.13 Network troubles "bisected"

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: <1959698732.20140326190752@eikelenboom.it>
Date:	Wed, 26 Mar 2014 19:07:52 +0100
From:	Sander Eikelenboom <linux@...elenboom.it>
To:	Paul Durrant <Paul.Durrant@...rix.com>
CC:	Wei Liu <wei.liu2@...rix.com>, annie li <annie.li@...cle.com>,
	Zoltan Kiss <zoltan.kiss@...rix.com>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [Xen-devel] Xen-unstable Linux 3.14-rc3 and 3.13 Network troubles "bisected"


Wednesday, March 26, 2014, 6:46:06 PM, you wrote:

> Re-send shortened version...

>> -----Original Message-----
>> From: Sander Eikelenboom [mailto:linux@...elenboom.it]
>> Sent: 26 March 2014 16:54
>> To: Paul Durrant
>> Cc: Wei Liu; annie li; Zoltan Kiss; xen-devel@...ts.xen.org; Ian Campbell; linux-
>> kernel; netdev@...r.kernel.org
>> Subject: Re: [Xen-devel] Xen-unstable Linux 3.14-rc3 and 3.13 Network
>> troubles "bisected"
>> 
> [snip]
>> >>
>> >> - When processing an SKB we end up in "xenvif_gop_frag_copy" while
>> prod
>> >> == cons ... but we still have bytes and size left ..
>> >> - start_new_rx_buffer() has returned true ..
>> >> - so we end up in get_next_rx_buffer
>> >> - this does a RING_GET_REQUEST and ups cons ..
>> >> - and we end up with a bad grant reference.
>> >>
>> >> Sometimes we are saved by the bell .. since additional slots have become
>> >> free (you see cons become > prod in "get_next_rx_buffer" but shortly
>> after
>> >> that prod is increased ..
>> >> just in time to not cause a overrun).
>> >>
>> 
>> > Ah, but hang on... There's a BUG_ON meta_slots_used >
>> max_slots_needed, so if we are overflowing the worst-case calculation then
>> why is that BUG_ON not firing?
>> 
>> You mean:
>>                 sco = (struct skb_cb_overlay *)skb->cb;
>>                 sco->meta_slots_used = xenvif_gop_skb(skb, &npo);
>>                 BUG_ON(sco->meta_slots_used > max_slots_needed);
>> 
>> in "get_next_rx_buffer" ?
>> 

> That code excerpt is from net_rx_action(),isn't it?

Yes

>> I don't know .. at least now it doesn't crash dom0 and therefore not my
>> complete machine and since tcp is recovering from a failed packet  :-)
>> 

> Well, if the code calculating max_slots_needed were underestimating then the BUG_ON() should fire. If it is not firing in your case then this suggests your problem lies elsewhere, or that meta_slots_used is not equal to the number of ring slots consumed.

It's seem to be the last ..

[ 1157.188908] vif vif-7-0 vif7.0: ?!? xenvif_gop_skb Me here 5 npo->meta_prod:40 old_meta_prod:36 vif->rx.sring->req_prod:2105867 vif->rx.req_cons:2105868 meta->gso_type:1 meta->gso_size:1448 nr_frags:1 req->gref:657 req->id:7 estimated_slots_needed:4 j(data):1 reserved_slots_left:-1    used in funcstart: 0 + 1 .. used_dataloop:1 .. used_fragloop:3
[ 1157.244975] vif vif-7-0 vif7.0: ?!? xenvif_rx_action me here 2 ..  vif->rx.sring->req_prod:2105867 vif->rx.req_cons:2105868 sco->meta_slots_used:4 max_upped_gso:1 skb_is_gso(skb):1 max_slots_needed:4 j:6 is_gso:1 nr_frags:1 firstpart:1 secondpart:2 reserved_slots_left:-1

net_rx_action() calculated we would need 4 slots .. and sco->meta_slots_used == 4 when we return so it doesn't trigger you BUG_ON ..

The 4 slots we calculated are:
  1 slot for the data part: DIV_ROUND_UP(offset_in_page(skb->data) + skb_headlen(skb), PAGE_SIZE)
  2 slots for the single frag in this SKB from: DIV_ROUND_UP(size, PAGE_SIZE)
  1 slot since GSO

In the debug code i annotated all cons++, and the code uses 1 slot to process the data from the SKB as expected but uses 3 slots in the frag chopping loop.
And when it reaches the state  were cons > prod it is always in "get_next_rx_buffer".

>> But probably because "npo->copy_prod++" seems to be used for the frags ..
>> and it isn't added to  npo->meta_prod ?
>> 

> meta_slots_used is calculated as the value of meta_prod at return (from xenvif_gop_skb()) minus the value on entry ,
> and if you look back up the code then you can see that meta_prod is incremented every time RING_GET_REQUEST() is evaluated.
> So, we must be consuming a slot without evaluating RING_GET_REQUEST() and I think that's exactly what's happening...
> Right at the bottom of xenvif_gop_frag_copy() req_cons is simply incremented in the case of a GSO. So the BUG_ON() is indeed off by one.

That is probably only done on first iteration / frag ?
Because i don't see my warn there trigger .. but it could be that's because at that moment we still have cons <= prod.


>   Paul



--
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