* Re: [PATCH net] xfrm: Release dst if this dst is improper for vti tunnel
From: David Miller @ 2013-11-19 20:51 UTC (permalink / raw)
To: fan.du; +Cc: steffen.klassert, saurabh.mohan, netdev
In-Reply-To: <1384851208-30353-1-git-send-email-fan.du@windriver.com>
From: Fan Du <fan.du@windriver.com>
Date: Tue, 19 Nov 2013 16:53:28 +0800
> After searching rt by the vti tunnel dst/src parameter,
> if this rt has neither attached to any transformation
> nor the transformation is not tunnel oriented, this rt
> should be released back to ip layer.
>
> otherwise causing dst memory leakage.
>
> Signed-off-by: Fan Du <fan.du@windriver.com>
Good catch, applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH] atm: idt77252: fix dev refcnt leak
From: David Miller @ 2013-11-19 20:53 UTC (permalink / raw)
To: ying.xue; +Cc: chas, linux-atm-general, netdev, linux-kernel
In-Reply-To: <1384855767-21446-1-git-send-email-ying.xue@windriver.com>
From: Ying Xue <ying.xue@windriver.com>
Date: Tue, 19 Nov 2013 18:09:27 +0800
> init_card() calls dev_get_by_name() to get a network deceive. But it
> doesn't decrease network device reference count after the device is
> used.
>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* [PATCH] net: core: Always propagate flag changes to interfaces
From: Vlad Yasevich @ 2013-11-19 21:00 UTC (permalink / raw)
To: netdev; +Cc: s.priebe, vfalico, Vlad Yasevich
The following commit:
b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
net: only invoke dev->change_rx_flags when device is UP
tried to fix a problem with VLAN devices and promiscuouse flag setting.
The issue was that VLAN device was setting a flag on an interface that
was down, thus resulting in bad promiscuity count.
This commit blocked flag propagation to any device that is currently
down.
A later commit:
deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
vlan: Don't propagate flag changes on down interfaces
fixed VLAN code to only propagate flags when the VLAN interface is up,
thus fixing the same issue as above, only localized to VLAN.
The problem we have now is that if we have create a complex stack
involving multiple software devices like bridges, bonds, and vlans,
then it is possible that the flags would not propagate properly to
the physical devices. A simple examle of the scenario is the
following:
eth0----> bond0 ----> bridge0 ---> vlan50
If bond0 or eth0 happen to be down at the time bond0 is added to
the bridge, then eth0 will never have promisc mode set which is
currently required for operation as part of the bridge. As a
result, packets with vlan50 will be dropped by the interface.
The only 2 devices that implement the special flag handling are
VLAN and DSA and they both have required code to prevent incorrect
flag propagation. As a result we can remove the generic solution
introduced in b6c40d68ff6498b7f63ddf97cf0aa818d748dee7 and leave
it to the individual devices to decide whether they will block
flag propagation or not.
Reported-by: Stefan Priebe <s.priebe@profihost.ag>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/core/dev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9a59f6a..da9c5e1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4991,9 +4991,7 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
{
const struct net_device_ops *ops = dev->netdev_ops;
- if (((dev->flags & IFF_UP) ||
- (dev->flags & (IFF_MASTER | IFF_SLAVE)))
- && ops->ndo_change_rx_flags)
+ if (ops->ndo_change_rx_flags)
ops->ndo_change_rx_flags(dev, flags);
}
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
From: David Miller @ 2013-11-19 21:10 UTC (permalink / raw)
To: vyasevich; +Cc: dborkman, netdev, linux-sctp, wangweidong1, eric.dumazet
In-Reply-To: <528B85E2.5080403@gmail.com>
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Tue, 19 Nov 2013 10:38:10 -0500
> This fixes the issue for the accept() case, but the bug is still there
> in the peeloff case.
>
> I think you should make sctp_copy_sock() use sock_copy() for now. It
> looks like it will catch all the cases.
>
> We can then look at possibly getting rid of sctp_copy_sock() for net-next.
Agreed, it looks like a lot of hair exists because sctp tries to use
custom code to copy sockets. Getting away from this will fix this
address problem as well as things like the pacing rate being wrong
as Eric Dumazet mentioned.
^ permalink raw reply
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: David Miller @ 2013-11-19 21:13 UTC (permalink / raw)
To: David.Laight; +Cc: mlord, eric.dumazet, ming.lei, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7431@saturn3.aculab.com>
From: "David Laight" <David.Laight@ACULAB.COM>
Date: Tue, 19 Nov 2013 10:04:11 -0000
> There is a patch to xhci-ring.c that should fix the SG problem.
> http://www.spinics.net/lists/linux-usb/msg97176.html
>
> I think it should apply to the 3.12 sources.
David, please get this into Linus's tree and queued up for -stable
as soon as possible, thank you.
^ permalink raw reply
* Re: [PATCH] tcp: don't update snd_nxt, when a socket is switched from repair mode
From: David Miller @ 2013-11-19 21:15 UTC (permalink / raw)
To: avagin
Cc: netdev, linux-kernel, criu, xemul, edumazet, kuznet, jmorris,
yoshfuji, kaber
In-Reply-To: <1384884606-9978-1-git-send-email-avagin@openvz.org>
From: Andrey Vagin <avagin@openvz.org>
Date: Tue, 19 Nov 2013 22:10:06 +0400
> snd_nxt must be updated synchronously with sk_send_head. Otherwise
> tp->packets_out may be updated incorrectly, what may bring a kernel panic.
>
> Here is a kernel panic from my host.
> [ 103.043194] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
> [ 103.044025] IP: [<ffffffff815aaaaf>] tcp_rearm_rto+0xcf/0x150
> ...
> [ 146.301158] Call Trace:
> [ 146.301158] [<ffffffff815ab7f0>] tcp_ack+0xcc0/0x12c0
>
> Before this panic a tcp socket was restored. This socket had sent and
> unsent data in the write queue. Sent data was restored in repair mode,
> then the socket was switched from reapair mode and unsent data was
> restored. After that the socket was switched back into repair mode.
>
> In that moment we had a socket where write queue looks like this:
> snd_una snd_nxt write_seq
> |_________|________|
> |
> sk_send_head
>
> After a second switching from repair mode the state of socket was
> changed:
>
> snd_una snd_nxt, write_seq
> |_________ ________|
> |
> sk_send_head
>
> This state is inconsistent, because snd_nxt and sk_send_head are not
> synchronized.
>
> Bellow you can find a call trace, how packets_out can be incremented
> twice for one skb, if snd_nxt and sk_send_head are not synchronized.
> In this case packets_out will be always positive, even when
> sk_write_queue is empty.
>
> tcp_write_wakeup
> skb = tcp_send_head(sk);
> tcp_fragment
> if (!before(tp->snd_nxt, TCP_SKB_CB(buff)->end_seq))
> tcp_adjust_pcount(sk, skb, diff);
> tcp_event_new_data_sent
> tp->packets_out += tcp_skb_pcount(skb);
>
> I think update of snd_nxt isn't required, when a socket is switched from
> repair mode. Because it's initialized in tcp_connect_init. Then when a
> write queue is restored, snd_nxt is incremented in tcp_event_new_data_sent,
> so it's always is in consistent state.
>
> I have checked, that the bug is not reproduced with this patch and
> all tests about restoring tcp connections work fine.
>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [patch net-next RFC v2 1/2] ipv6 addrconf: extend ifa_flags to u32
From: David Miller @ 2013-11-19 21:16 UTC (permalink / raw)
To: jiri
Cc: netdev, kuznet, jmorris, yoshfuji, kaber, thaller, stephen,
hannes, vyasevich, dcbw, David.Laight
In-Reply-To: <1384857809-6573-2-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 19 Nov 2013 11:43:28 +0100
> There is no more space in u8 ifa_flags. So do what davem suffested and
> add another netlink attr called IFA_FLAGS for carry more flags.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Thomas Haller <thaller@redhat.com>
> ---
> v1->v2:
> - reordered struct inet6_ifaddr as suggested by David Laight
> - changed flags type to u32 on couple of places as suggested by Thomas Haller
> - changed output of if6_seq_show as suggested by Thomas Haller
This looks fine, but I would suggest to extend this to all addressing
types using ifm->ifa_flags. It is not such a big deal, there are only
3 or 4.
DecNET, ipv4, ipv6, and lastly phonet (which just reports "permanent"
flag on get/dump).
That way tools like iproute2 can just generically provide and
interpret IFA_FLAGS attribute unconditionally.
^ permalink raw reply
* Re: [PATCH] net: core: Always propagate flag changes to interfaces
From: David Miller @ 2013-11-19 21:24 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, s.priebe, vfalico
In-Reply-To: <1384894858-3238-1-git-send-email-vyasevic@redhat.com>
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 19 Nov 2013 16:00:58 -0500
> @@ -4991,9 +4991,7 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
>
> - if (((dev->flags & IFF_UP) ||
> - (dev->flags & (IFF_MASTER | IFF_SLAVE)))
> - && ops->ndo_change_rx_flags)
> + if (ops->ndo_change_rx_flags)
> ops->ndo_change_rx_flags(dev, flags);
Right now this test looks like:
if ((dev->flags & IFF_UP) && ops->ndo_change_rx_flags)
in 'net' tree, please respin against current sources.
Thanks.
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Eric Dumazet @ 2013-11-19 21:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <20131119204909.GA15004@redhat.com>
On Tue, 2013-11-19 at 22:49 +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 19, 2013 at 06:03:48AM -0800, Eric Dumazet wrote:
> > On Tue, 2013-11-19 at 16:05 +0800, Jason Wang wrote:
> > > We need to drop the refcnt of page when we fail to allocate an skb for frag
> > > list, otherwise it will be leaked. The bug was introduced by commit
> > > 2613af0ed18a11d5c566a81f9a6510b73180660a ("virtio_net: migrate mergeable rx
> > > buffers to page frag allocators").
> > >
> > > Cc: Michael Dalton <mwdalton@google.com>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > > The patch was needed for 3.12 stable.
> >
> > Good catch, but if we return from receive_mergeable() in the 'middle'
> > of the frags we would need for the current skb, who will
> > call the virtqueue_get_buf() to flush the remaining frags ?
> >
> > Don't we also need to call virtqueue_get_buf() like
> >
> > while (--num_buf) {
> > buf = virtqueue_get_buf(rq->vq, &len);
> > if (!buf)
> > break;
> > put_page(virt_to_head_page(buf));
> > }
> >
> > ?
> >
> >
>
>
> Let me explain what worries me in your suggestion:
>
> struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> if (unlikely(!nskb)) {
> head_skb->dev->stats.rx_dropped++;
> return -ENOMEM;
> }
>
> is this the failure case we are talking about?
I thought Jason patch was about this, no ?
>
> I think this is a symprom of a larger problem
> introduced by 2613af0ed18a11d5c566a81f9a6510b73180660a,
> namely that we now need to allocate memory in the
> middle of processing a packet.
>
>
> I think discarding a completely valid and well-formed
> packet from the receive queue because we are unable
> to allocate new memory with GFP_ATOMIC
> for future packets is not a good idea.
How is it different with NIC processing in RX path ?
>
> It certainly violates the principle of least surprize:
> when one sees host pass packet to guest, one expects
> the packet to get into the networking stack, not get
> dropped by the driver internally.
> Guest stack can do with the packet what it sees fit.
>
> We actually wake up a thread if we can't fill up the queue,
> that will fill it up in GFP_KERNEL context.
>
> So I think we should find a way to pre-allocate if necessary and avoid
> error paths where allocating new memory is a required to avoid drops.
>
Really, under ATOMIC context, there is no way you can avoid dropping
packets if you cannot allocate memory. If you cannot allocate sk_buff
(256 bytes !!), you wont be able to allocate the 1500+ bytes to hold the
payload of next packets anyway.
Same problem on a real NIC.
Under memory pressure we _do_ packet drops.
Nobody really complained.
Sure, you can add yet another cache of pre-allocated skbs and pay the
price of managing yet another cache layer, but still need to trop
packets under stress.
Pre-allocating skb on real NIC has a performance cost, because we clear
sk_buff way ahead of time. By the time skb is finally received, cpu has
to bring back into its cache memory cache lines.
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Michael Dalton @ 2013-11-19 21:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric Dumazet, netdev, linux-kernel, lf-virt, Eric Dumazet
In-Reply-To: <20131119204909.GA15004@redhat.com>
Great catch Jason. I agree this now raises the larger issue of how to
handle a memory alloc failure in the middle of receive. As Eric mentioned,
we can drop the packet and free the remaining (num_buf) frags.
Michael, perhaps I'm missing something, but why would you prefer
pre-allocating buffers in this case? If the guest kernel is OOM'ing,
dropping packets should provide backpressure.
Also, we could just as easily fail the initial skb alloc in page_to_skb,
and I think that case also needs to be handled now in the same fashion as
a memory allocation failure in receive_mergeable.
Best,
Mike
^ permalink raw reply
* Re: [PATCH 0/9] genetlink: clean up multicast group APIs
From: David Miller @ 2013-11-19 21:40 UTC (permalink / raw)
To: johannes; +Cc: netdev
In-Reply-To: <1384870779-17571-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 19 Nov 2013 15:19:30 +0100
> The generic netlink multicast group registration doesn't have to
> be dynamic, and can thus be simplified just like I did with the
> ops. This removes some complexity in registration code.
>
> Additionally, two users of generic netlink already use multicast
> groups in a wrong way, add workarounds for those two to keep the
> userspace API working, but at the same time make them not clash
> with other users of multicast groups as might happen now.
>
> While making it all a bit easier, also prevent such abuse by adding
> checks to the APIs so each family can only use the groups it owns.
This looks great, and makes the unfortunate multicast ID handling in
DM and elsewhere explicit, controlled, and validated.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Michael S. Tsirkin @ 2013-11-19 21:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <1384896996.8604.120.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 01:36:36PM -0800, Eric Dumazet wrote:
> On Tue, 2013-11-19 at 22:49 +0200, Michael S. Tsirkin wrote:
> > On Tue, Nov 19, 2013 at 06:03:48AM -0800, Eric Dumazet wrote:
> > > On Tue, 2013-11-19 at 16:05 +0800, Jason Wang wrote:
> > > > We need to drop the refcnt of page when we fail to allocate an skb for frag
> > > > list, otherwise it will be leaked. The bug was introduced by commit
> > > > 2613af0ed18a11d5c566a81f9a6510b73180660a ("virtio_net: migrate mergeable rx
> > > > buffers to page frag allocators").
> > > >
> > > > Cc: Michael Dalton <mwdalton@google.com>
> > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > ---
> > > > The patch was needed for 3.12 stable.
> > >
> > > Good catch, but if we return from receive_mergeable() in the 'middle'
> > > of the frags we would need for the current skb, who will
> > > call the virtqueue_get_buf() to flush the remaining frags ?
> > >
> > > Don't we also need to call virtqueue_get_buf() like
> > >
> > > while (--num_buf) {
> > > buf = virtqueue_get_buf(rq->vq, &len);
> > > if (!buf)
> > > break;
> > > put_page(virt_to_head_page(buf));
> > > }
> > >
> > > ?
> > >
> > >
> >
> >
> > Let me explain what worries me in your suggestion:
> >
> > struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> > if (unlikely(!nskb)) {
> > head_skb->dev->stats.rx_dropped++;
> > return -ENOMEM;
> > }
> >
> > is this the failure case we are talking about?
>
> I thought Jason patch was about this, no ?
>
> >
> > I think this is a symprom of a larger problem
> > introduced by 2613af0ed18a11d5c566a81f9a6510b73180660a,
> > namely that we now need to allocate memory in the
> > middle of processing a packet.
> >
> >
> > I think discarding a completely valid and well-formed
> > packet from the receive queue because we are unable
> > to allocate new memory with GFP_ATOMIC
> > for future packets is not a good idea.
>
> How is it different with NIC processing in RX path ?
Which NIC? Virtio? Prior to 2613af0ed18a11d5c566a81f9a6510b73180660a
it didn't drop packets received from host as far as I can tell.
virtio is more like a pipe than a real NIC in this respect.
> >
> > It certainly violates the principle of least surprize:
> > when one sees host pass packet to guest, one expects
> > the packet to get into the networking stack, not get
> > dropped by the driver internally.
> > Guest stack can do with the packet what it sees fit.
> >
> > We actually wake up a thread if we can't fill up the queue,
> > that will fill it up in GFP_KERNEL context.
> >
> > So I think we should find a way to pre-allocate if necessary and avoid
> > error paths where allocating new memory is a required to avoid drops.
> >
>
> Really, under ATOMIC context, there is no way you can avoid dropping
> packets if you cannot allocate memory. If you cannot allocate sk_buff
> (256 bytes !!), you wont be able to allocate the 1500+ bytes to hold the
> payload of next packets anyway.
that's why we do:
if (!try_fill_recv(rq, GFP_ATOMIC))
schedule_delayed_work(&vi->refill, 0);
the queues are large enough for a single failure not to be
an immediate problem.
> Same problem on a real NIC.
>
> Under memory pressure we _do_ packet drops.
> Nobody really complained.
>
> Sure, you can add yet another cache of pre-allocated skbs and pay the
> price of managing yet another cache layer, but still need to trop
> packets under stress.
We don't need a cache even. Just enough to avoid dropping packets
if allocation failed in the middle so we don't dequeue a buffer and then
drop it.
Once we use this reserved skb, we stop processing the queue until
refill gives it back.
> Pre-allocating skb on real NIC has a performance cost, because we clear
> sk_buff way ahead of time. By the time skb is finally received, cpu has
> to bring back into its cache memory cache lines.
>
Alternatively we can pre-allocate the memory but avoid clearing it maybe?
--
MST
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Eric Dumazet @ 2013-11-19 22:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <20131119215312.GE15004@redhat.com>
On Tue, 2013-11-19 at 23:53 +0200, Michael S. Tsirkin wrote:
> Which NIC? Virtio? Prior to 2613af0ed18a11d5c566a81f9a6510b73180660a
> it didn't drop packets received from host as far as I can tell.
> virtio is more like a pipe than a real NIC in this respect.
Prior/after to this patch, you were not posting buffers, so if packets
were received on a physical NIC, you were dropping the packets anyway.
It makes no difference at all, adding a cushion might make you feel
better, but its really not worth it.
Under memory stress, it makes better sense to drop a super big GRO
packet (The one needing frag_list extension ...)
It gives a better signal to the sender to reduce its pressure, and gives
opportunity to free more of your memory.
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2013-11-19 22:18 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
Mostly these are fixes for fallout due to merge window changes, as well as
cures for problems that have been with us for a much longer period of time.
1) Johannes Berg noticed two major deficiencies in our genetlink registration.
Some genetlink protocols we passing in constant counts for their ops array
rather than something like ARRAY_SIZE(ops) or similar. Also, some genetlink
protocols were using fixed IDs for their multicast groups.
We have to retain these fixed IDs to keep existing userland tools
working, but reserve them so that other multicast groups used by
other protocols can not possibly conflict.
In dealing with these two problems, we actually now use less state
management for genetlink operations and multicast groups.
2) When configuring interface hardware timestamping, fix several drivers
that simply do not validate that the hwtstamp_config value is one the
driver actually supports. From Ben Hutchings.
3) Invalid memory references in mwifiex driver, from Amitkumar Karwar.
4) In dev_forward_skb(), set the skb->protocol in the right order relative
to skb_scrub_packet(). From Alexei Starovoitov.
5) Bridge erroneously fails to use the proper wrapper functions to
make calls to netdev_ops->ndo_vlan_rx_{add,kill}_vid. Fix from
Toshiaki Makita.
6) When detaching a bridge port, make sure to flush all VLAN IDs to
prevent them from leaking, also from Toshiaki Makita.
7) Put in a compromise for TCP Small Queues so that deep queued devices
that delay TX reclaim non-trivially don't have such a performance
decrease. One particularly problematic area is 802.11 AMPDU in wireless.
From Eric Dumazet.
8) Fix crashes in tcp_fastopen_cache_get(), we can see NULL socket dsts
here. Fix from Eric Dumzaet, reported by Dave Jones.
9) Fix use after free in ipv6 SIT driver, from Willem de Bruijn.
10) When computing mergeable buffer sizes, virtio-net fails to take the
virtio-net header into account. From Michael Dalton.
11) Fix seqlock deadlock in ip4_datagram_connect() wrt. statistic bumping,
this one has been with us for a while. From Eric Dumazet.
12) Fix NULL deref in the new TIPC fragmentation handling, from Erik
Hugne.
13) 6lowpan bit used for traffic classification was wrong, from
Jukka Rissanen.
14) macvlan has the same issue as normal vlans did wrt. propagating
LRO disabling down to the real device, fix it the same way. From
Michal Kubecek.
15) CPSW driver needs to soft reset all slaves during suspend, from
Daniel Mack.
16) Fix small frame pacing in FQ packet scheduler, from Eric Dumazet.
17) The xen-netfront RX buffer refill timer isn't properly scheduled
on partial RX allocation success, from Ma JieYue.
18) When ipv6 ping protocol support was added, the AF_INET6 protocol
initialization cleanup path on failure was borked a little. Fix
from Vlad Yasevich.
19) If a socket disconnects during a read/recvmsg/recvfrom/etc. that blocks
we can do the wrong thing with the msg_name we write back to userspace.
From Hannes Frederic Sowa. There is another fix in the works from
Hannes which will prevent future problems of this nature.
20) Fix route leak in VTI tunnel transmit, from Fan Du.
Please pull, thanks a lot!
The following changes since commit 42a2d923cc349583ebf6fdd52a7d35e1c2f7e6bd:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next (2013-11-13 17:40:34 +0900)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
for you to fetch changes up to 091e0662ee2c37867ad918ce7b6ddd17f0e090e2:
Merge branch 'genetlink_mcast' (2013-11-19 16:39:42 -0500)
----------------------------------------------------------------
Ajit Khaparde (2):
be2net: Fix unconditional enabling of Rx interface options
be2net: Delete secondary unicast MAC addresses during be_close
Alexei Starovoitov (1):
core/dev: do not ignore dmac in dev_forward_skb()
Amitkumar Karwar (4):
mwifiex: replace u16 with __le16 in struct mwifiex_types_power_group
mwifiex: fix invalid memory access in mwifiex_get_power_level()
mwifiex: fix invalid memory access in mwifiex_ret_tx_rate_cfg()
mwifiex: fix invalid memory access in mwifiex_update_autoindex_ies()
Andrey Vagin (1):
tcp: don't update snd_nxt, when a socket is switched from repair mode
Avinash Patil (1):
mwifiex: correct packet length for packets from SDIO interface
Ben Hutchings (6):
tg3: Validate hwtstamp_config completely before applying it
e1000e: Validate hwtstamp_config completely before applying it
pch_gbe: Validate hwtstamp_config completely before applying it
stmmac: Validate hwtstamp_config completely before applying it
ti_cpsw: Validate hwtstamp_config completely before applying it
ixp4xx_eth: Validate hwtstamp_config completely before applying it
Chang Xiangzhong (1):
net: sctp: bug-fixing: retran_path not set properly after transports recovering (v3)
Chris Metcalf (1):
connector: improved unaligned access error fix
Colin Ian King (1):
rtlwifi: fix null dereference on efuse_word on kmalloc fail returns NULL
Dan Carpenter (7):
mwifiex: potential integer underflow in mwifiex_ret_wmm_get_status()
libertas: potential oops in debugfs
wcn36xx: harmless memory corruption bug in debugfs
wcn36xx: missing unlocks on error paths
net: cdc_ncm: cleanup a type issue in cdc_ncm_setup()
net: mv643xx_eth: potential NULL dereference in probe()
isdnloop: use strlcpy() instead of strcpy()
Dan Williams (1):
prism54: set netdev type to "wlan"
Daniel Borkmann (2):
random32: add __init prefix to prandom_start_seed_timer
random32: use msecs_to_jiffies for reseed timer
Daniel Mack (1):
net: ethernet: ti/cpsw: do not crash on single-MAC machines during resume
David S. Miller (7):
Merge branch 'hwtstamp'
Merge branch 'genetlink'
Merge branch 'for-davem' of git://git.kernel.org/.../linville/wireless
Merge branch 'macvlan'
net: Handle CHECKSUM_COMPLETE more adequately in pskb_trim_rcsum().
Merge branch 'bnx2x'
Merge branch 'genetlink_mcast'
Divy Le Ray (1):
MAINTAINERS: cxgb3: Update cxgb3 maintainer entry
Dmitry Kravkov (4):
bnx2x: Clean the sp rtnl task upon unload
bnx2x: Prevent panic during DMAE timeout
bnx2x: prevent CFC attention
bnx2x: Prevent "timeout waiting for state X"
Duan Fugang-B38611 (1):
net:fec: fix WARNING caused by lack of calls to dma_mapping_error()
Eric Dumazet (5):
tcp: tsq: restore minimal amount of queueing
net-tcp: fix panic in tcp_fastopen_cache_set()
ipv4: fix possible seqlock deadlock
pkt_sched: fq: warn users using defrate
pkt_sched: fq: fix pacing for small frames
Erik Hugne (1):
tipc: fix dereference before check warning
Fabio Estevam (1):
net: ipv6: ndisc: Fix warning when CONFIG_SYSCTL=n
Felipe Pena (2):
wireless: rt2800lib: Fix typo on checking
rtlwifi: rtl8192se: Fix wrong assignment
Felix Fietkau (1):
usbnet: fix status interrupt urb handling
Geyslan G. Bem (1):
net/hsr: Fix possible leak in 'hsr_get_node_status()'
Hannes Frederic Sowa (2):
inet: prevent leakage of uninitialized memory to user in recv syscalls
ping: prevent NULL pointer dereference on write to msg_name
Janusz Dziedzic (2):
ath9k: DFS radar use correct width enum
ath9k: dfs_debug fix possible NULL dereference
Jason Wang (2):
tuntap: limit head length of skb allocated
macvtap: limit head length of skb allocated
Johannes Berg (21):
taskstats: use genl_register_family_with_ops()
hsr: use genl_register_family_with_ops()
ieee802154: use genl_register_family_with_ops()
wimax: use genl_register_family_with_ops()
genetlink: remove genl_register_ops/genl_unregister_ops
genetlink: register family ops as array
genetlink: allow making ops const
genetlink: make all genl_ops users const
genetlink: make genl_ops flags a u8 and move to end
genetlink: unify registration functions
genetlink: rename shadowed variable
netlink: fix documentation typo in netlink_set_err()
genetlink: only pass array to genl_register_family_with_ops()
drop_monitor/genetlink: use proper genetlink multicast APIs
quota/genetlink: use proper genetlink multicast APIs
hsr: don't call genl_unregister_mc_group()
genetlink: remove genl_unregister_mc_group()
genetlink: remove family pointer from genl_multicast_group
genetlink: add and use genl_set_err()
genetlink: pass family to functions using groups
genetlink: make multicast groups const, prevent abuse
John W. Linville (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
Jukka Rissanen (1):
6lowpan: Uncompression of traffic class field was incorrect
Larry Finger (3):
rtlwifi: rtl8192se: Fix incorrect signal strength for unassociated AP
rtlwifi: rtl8192cu: Fix incorrect signal strength for unassociated AP
rtlwifi: rtl8192de: Fix incorrect signal strength for unassociated AP
Ma JieYue (1):
xen-netfront: fix missing rx_refill_timer when allocate memory failed
Maciej Żenczykowski (1):
pkt_sched: fq: change classification of control packets
Mark Cave-Ayland (1):
rtlwifi: Fix endian error in extracting packet type
Michael Dalton (1):
virtio-net: mergeable buffer size should include virtio-net header
Michal Kubeček (2):
macvlan: introduce macvlan_dev_real_dev() helper function
macvlan: disable LRO on lower device instead of macvlan
Nicolas Dichtel (4):
sit: fix prefix length of ll and v4mapped addresses
sit: link local routes are missing
sit/gre6: don't try to add the same route two times
ip6tnl: fix use after free of fb_tnl_dev
Nikolay Aleksandrov (1):
bonding: fix two race conditions in bond_store_updelay/downdelay
Petko Manolov (1):
MAINTAINERS: Update Pegasus and RTL8150 repositories;
Stanislaw Gruszka (1):
rt2x00: fix HT TX descriptor settings regression
Sujith Manoharan (1):
ath9k: Use correct PCIE initvals for AR9485
Toshiaki Makita (3):
bridge: Use vlan_vid_[add/del] instead of direct ndo_vlan_rx_[add/kill]_vid calls
bridge: Call vlan_vid_del for all vids at nbp_vlan_flush
bridge: Fix memory leak when deleting bridge with vlan filtering enabled
Ujjal Roy (1):
mwifiex: fix wrong eth_hdr usage for bridged packets in AP mode
Veaceslav Falico (1):
bonding: don't permit to use ARP monitoring in 802.3ad mode
Vlad Yasevich (1):
ipv6: Fix inet6_init() cleanup order
Wang Weidong (1):
bonding: add ip checks when store ip target
Wei Yongjun (2):
libertas: fix error return code in if_cs_probe()
wcn36xx: Add missing unlock before return
Willem de Bruijn (1):
sit: fix use after free of fb_tunnel_dev
Ying Xue (1):
atm: idt77252: fix dev refcnt leak
Zhi Yong Wu (1):
net, virtio_net: replace the magic value
fan.du (1):
xfrm: Release dst if this dst is improper for vti tunnel
hahnjo (1):
alx: Reset phy speed after resume
Documentation/networking/ip-sysctl.txt | 3 -
MAINTAINERS | 10 +-
drivers/acpi/event.c | 25 ++--
drivers/atm/idt77252.c | 2 +-
drivers/connector/cn_proc.c | 72 +++++-----
drivers/isdn/isdnloop/isdnloop.c | 8 +-
drivers/net/bonding/bond_sysfs.c | 30 ++---
drivers/net/bonding/bonding.h | 3 +
drivers/net/ethernet/atheros/alx/main.c | 3 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 1 -
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c | 5 -
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 17 ++-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h | 11 ++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 2 +-
drivers/net/ethernet/broadcom/tg3.c | 16 +--
drivers/net/ethernet/emulex/benet/be_cmds.c | 16 ++-
drivers/net/ethernet/emulex/benet/be_cmds.h | 2 +-
drivers/net/ethernet/emulex/benet/be_main.c | 7 +-
drivers/net/ethernet/freescale/fec_main.c | 31 ++++-
drivers/net/ethernet/intel/e1000e/netdev.c | 14 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 3 +-
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 12 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +-
drivers/net/ethernet/ti/cpsw.c | 36 ++---
drivers/net/ethernet/xscale/ixp4xx_eth.c | 12 +-
drivers/net/macvtap.c | 8 +-
drivers/net/team/team.c | 29 +---
drivers/net/tun.c | 10 +-
drivers/net/usb/cdc_ncm.c | 2 +-
drivers/net/usb/usbnet.c | 3 -
drivers/net/virtio_net.c | 34 ++---
drivers/net/wireless/ath/ath9k/ar9003_hw.c | 22 +--
drivers/net/wireless/ath/ath9k/ar9485_initvals.h | 42 ++----
drivers/net/wireless/ath/ath9k/ath9k.h | 19 +--
drivers/net/wireless/ath/ath9k/dfs_debug.c | 13 +-
drivers/net/wireless/ath/ath9k/hw.h | 1 +
drivers/net/wireless/ath/ath9k/init.c | 9 +-
drivers/net/wireless/ath/ath9k/pci.c | 87 ++++++++++++
drivers/net/wireless/ath/wcn36xx/debug.c | 2 +-
drivers/net/wireless/ath/wcn36xx/smd.c | 9 +-
drivers/net/wireless/libertas/debugfs.c | 6 +-
drivers/net/wireless/libertas/if_cs.c | 1 +
drivers/net/wireless/mac80211_hwsim.c | 5 +-
drivers/net/wireless/mwifiex/fw.h | 4 +-
drivers/net/wireless/mwifiex/ie.c | 11 +-
drivers/net/wireless/mwifiex/sdio.c | 3 +
drivers/net/wireless/mwifiex/sta_cmd.c | 4 +-
drivers/net/wireless/mwifiex/sta_cmdresp.c | 46 ++++---
drivers/net/wireless/mwifiex/sta_ioctl.c | 5 +-
drivers/net/wireless/mwifiex/uap_txrx.c | 29 +++-
drivers/net/wireless/mwifiex/wmm.c | 3 +
drivers/net/wireless/prism54/islpci_dev.c | 7 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/rt2x00/rt2x00lib.h | 2 +-
drivers/net/wireless/rt2x00/rt2x00mac.c | 4 +-
drivers/net/wireless/rt2x00/rt2x00queue.c | 4 +-
drivers/net/wireless/rtlwifi/base.c | 93 ++++++-------
drivers/net/wireless/rtlwifi/efuse.c | 5 +-
drivers/net/wireless/rtlwifi/rtl8192cu/trx.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192de/trx.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192se/rf.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 2 +-
drivers/net/wireless/rtlwifi/wifi.h | 6 +-
drivers/net/xen-netfront.c | 7 +-
drivers/scsi/pmcraid.c | 3 +-
drivers/thermal/thermal_core.c | 25 ++--
fs/dlm/netlink.c | 10 +-
fs/quota/netlink.c | 16 ++-
include/linux/genl_magic_func.h | 53 +++-----
include/linux/if_macvlan.h | 17 +++
include/linux/skbuff.h | 39 +++---
include/net/genetlink.h | 131 ++++++++++++------
include/uapi/linux/genetlink.h | 1 +
include/uapi/linux/pkt_sched.h | 7 +-
kernel/taskstats.c | 38 ++----
lib/random32.c | 12 +-
net/bridge/br_if.c | 1 +
net/bridge/br_vlan.c | 24 ++--
net/core/dev.c | 11 +-
net/core/drop_monitor.c | 15 ++-
net/hsr/hsr_netlink.c | 64 +++------
net/ieee802154/6lowpan.c | 4 +-
net/ieee802154/dgram.c | 3 +-
net/ieee802154/ieee802154.h | 21 ++-
net/ieee802154/netlink.c | 45 ++++---
net/ieee802154/nl-mac.c | 79 ++---------
net/ieee802154/nl-phy.c | 37 +-----
net/ipv4/datagram.c | 2 +-
net/ipv4/ip_tunnel.c | 4 +-
net/ipv4/ip_vti.c | 1 +
net/ipv4/ping.c | 49 ++++---
net/ipv4/raw.c | 4 +-
net/ipv4/tcp.c | 6 -
net/ipv4/tcp_metrics.c | 10 +-
net/ipv4/tcp_output.c | 7 +-
net/ipv4/udp.c | 7 +-
net/ipv6/addrconf.c | 38 ++----
net/ipv6/af_inet6.c | 4 +-
net/ipv6/ip6_tunnel.c | 18 ++-
net/ipv6/ndisc.c | 2 +-
net/ipv6/raw.c | 4 +-
net/ipv6/sit.c | 18 ++-
net/ipv6/udp.c | 5 +-
net/irda/irnetlink.c | 5 +-
net/l2tp/l2tp_ip.c | 4 +-
net/l2tp/l2tp_netlink.c | 9 +-
net/netfilter/ipvs/ip_vs_ctl.c | 4 +-
net/netlabel/netlabel_cipso_v4.c | 4 +-
net/netlabel/netlabel_mgmt.c | 4 +-
net/netlabel/netlabel_unlabeled.c | 4 +-
net/netlink/af_netlink.c | 2 +-
net/netlink/genetlink.c | 524 ++++++++++++++++++++++++++++++++++--------------------------------------
net/nfc/netlink.c | 41 +++---
net/openvswitch/datapath.c | 59 ++++----
net/openvswitch/datapath.h | 1 +
net/openvswitch/dp_notify.c | 11 +-
net/phonet/datagram.c | 9 +-
net/sched/sch_fq.c | 40 +++---
net/sctp/associola.c | 6 +-
net/tipc/link.c | 3 +-
net/tipc/netlink.c | 11 +-
net/wimax/op-msg.c | 27 +---
net/wimax/op-reset.c | 17 ---
net/wimax/op-rfkill.c | 21 ---
net/wimax/op-state-get.c | 17 ---
net/wimax/stack.c | 95 +++++++------
net/wimax/wimax-internal.h | 8 +-
net/wireless/nl80211.c | 206 +++++++++++++---------------
129 files changed, 1437 insertions(+), 1432 deletions(-)
^ permalink raw reply
* Re: ipv6 fragmentation-related panic in netfilter
From: Wolfgang Walter @ 2013-11-19 22:27 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Patrick McHardy, Steffen Klassert, Tomas Hlavacek, netdev,
netfilter-devel, David Miller
In-Reply-To: <20131119124032.GS16541@order.stressinduktion.org>
Am Dienstag, 19. November 2013, 13:40:32 schrieb Hannes Frederic Sowa:
> On Tue, Nov 19, 2013 at 12:11:24PM +0100, Wolfgang Walter wrote:
> > Are there patches available? I can crash my 3.12 kernel easily doing
> >
> > fping -p 20 -l -b 4000 bla
> >
> > 3.11.x does not expose this problem.
>
> Yes, see here:
>
> http://patchwork.ozlabs.org/patch/288967/
> http://patchwork.ozlabs.org/patch/288970/
>
This fixed it. The second patch did not cleanly apply to 3.12 due to
formatting changes or because some functions in 3.12 get
unsigned int hooknum
instead of
const struct nf_hook_ops *ops
I hopefully got it right:at least it works and no crashes any more :-).
I hope both patches go into stable soon.
Thanks,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Here is the modified version of the second patch if someone needs it (but no guarantee that it is correct):
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c2d8933..f66f346 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -333,11 +333,6 @@ typedef unsigned int sk_buff_data_t;
typedef unsigned char *sk_buff_data_t;
#endif
-#if defined(CONFIG_NF_DEFRAG_IPV4) || defined(CONFIG_NF_DEFRAG_IPV4_MODULE) || \
- defined(CONFIG_NF_DEFRAG_IPV6) || defined(CONFIG_NF_DEFRAG_IPV6_MODULE)
-#define NET_SKBUFF_NF_DEFRAG_NEEDED 1
-#endif
-
/**
* struct sk_buff - socket buffer
* @next: Next buffer in list
@@ -370,7 +365,6 @@ typedef unsigned char *sk_buff_data_t;
* @protocol: Packet protocol from driver
* @destructor: Destruct function
* @nfct: Associated connection, if any
- * @nfct_reasm: netfilter conntrack re-assembly pointer
* @nf_bridge: Saved data about a bridged frame - see br_netfilter.c
* @skb_iif: ifindex of device we arrived on
* @tc_index: Traffic control index
@@ -459,9 +453,6 @@ struct sk_buff {
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
struct nf_conntrack *nfct;
#endif
-#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
- struct sk_buff *nfct_reasm;
-#endif
#ifdef CONFIG_BRIDGE_NETFILTER
struct nf_bridge_info *nf_bridge;
#endif
@@ -2605,18 +2596,6 @@ static inline void nf_conntrack_get(struct nf_conntrack *nfct)
atomic_inc(&nfct->use);
}
#endif
-#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
-static inline void nf_conntrack_get_reasm(struct sk_buff *skb)
-{
- if (skb)
- atomic_inc(&skb->users);
-}
-static inline void nf_conntrack_put_reasm(struct sk_buff *skb)
-{
- if (skb)
- kfree_skb(skb);
-}
-#endif
#ifdef CONFIG_BRIDGE_NETFILTER
static inline void nf_bridge_put(struct nf_bridge_info *nf_bridge)
{
@@ -2635,10 +2614,6 @@ static inline void nf_reset(struct sk_buff *skb)
nf_conntrack_put(skb->nfct);
skb->nfct = NULL;
#endif
-#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
- nf_conntrack_put_reasm(skb->nfct_reasm);
- skb->nfct_reasm = NULL;
-#endif
#ifdef CONFIG_BRIDGE_NETFILTER
nf_bridge_put(skb->nf_bridge);
skb->nf_bridge = NULL;
@@ -2660,10 +2635,6 @@ static inline void __nf_copy(struct sk_buff *dst, const struct sk_buff *src)
nf_conntrack_get(src->nfct);
dst->nfctinfo = src->nfctinfo;
#endif
-#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
- dst->nfct_reasm = src->nfct_reasm;
- nf_conntrack_get_reasm(src->nfct_reasm);
-#endif
#ifdef CONFIG_BRIDGE_NETFILTER
dst->nf_bridge = src->nf_bridge;
nf_bridge_get(src->nf_bridge);
@@ -2675,9 +2646,6 @@ static inline void nf_copy(struct sk_buff *dst, const struct sk_buff *src)
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
nf_conntrack_put(dst->nfct);
#endif
-#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
- nf_conntrack_put_reasm(dst->nfct_reasm);
-#endif
#ifdef CONFIG_BRIDGE_NETFILTER
nf_bridge_put(dst->nf_bridge);
#endif
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 9c4d37e..772252d 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -109,7 +109,6 @@ extern int ip_vs_conn_tab_size;
struct ip_vs_iphdr {
__u32 len; /* IPv4 simply where L4 starts
IPv6 where L4 Transport Header starts */
- __u32 thoff_reasm; /* Transport Header Offset in nfct_reasm skb */
__u16 fragoffs; /* IPv6 fragment offset, 0 if first frag (or not frag)*/
__s16 protocol;
__s32 flags;
@@ -117,34 +116,12 @@ struct ip_vs_iphdr {
union nf_inet_addr daddr;
};
-/* Dependency to module: nf_defrag_ipv6 */
-#if defined(CONFIG_NF_DEFRAG_IPV6) || defined(CONFIG_NF_DEFRAG_IPV6_MODULE)
-static inline struct sk_buff *skb_nfct_reasm(const struct sk_buff *skb)
-{
- return skb->nfct_reasm;
-}
-static inline void *frag_safe_skb_hp(const struct sk_buff *skb, int offset,
- int len, void *buffer,
- const struct ip_vs_iphdr *ipvsh)
-{
- if (unlikely(ipvsh->fragoffs && skb_nfct_reasm(skb)))
- return skb_header_pointer(skb_nfct_reasm(skb),
- ipvsh->thoff_reasm, len, buffer);
-
- return skb_header_pointer(skb, offset, len, buffer);
-}
-#else
-static inline struct sk_buff *skb_nfct_reasm(const struct sk_buff *skb)
-{
- return NULL;
-}
static inline void *frag_safe_skb_hp(const struct sk_buff *skb, int offset,
int len, void *buffer,
const struct ip_vs_iphdr *ipvsh)
{
return skb_header_pointer(skb, offset, len, buffer);
}
-#endif
static inline void
ip_vs_fill_ip4hdr(const void *nh, struct ip_vs_iphdr *iphdr)
@@ -171,19 +148,12 @@ ip_vs_fill_iph_skb(int af, const struct sk_buff *skb, struct ip_vs_iphdr *iphdr)
(struct ipv6hdr *)skb_network_header(skb);
iphdr->saddr.in6 = iph->saddr;
iphdr->daddr.in6 = iph->daddr;
- /* ipv6_find_hdr() updates len, flags, thoff_reasm */
- iphdr->thoff_reasm = 0;
+ /* ipv6_find_hdr() updates len, flags */
iphdr->len = 0;
iphdr->flags = 0;
iphdr->protocol = ipv6_find_hdr(skb, &iphdr->len, -1,
&iphdr->fragoffs,
&iphdr->flags);
- /* get proto from re-assembled packet and it's offset */
- if (skb_nfct_reasm(skb))
- iphdr->protocol = ipv6_find_hdr(skb_nfct_reasm(skb),
- &iphdr->thoff_reasm,
- -1, NULL, NULL);
-
} else
#endif
{
diff --git a/include/net/netfilter/ipv6/nf_defrag_ipv6.h b/include/net/netfilter/ipv6/nf_defrag_ipv6.h
index fd79c9a..80a3f41 100644
--- a/include/net/netfilter/ipv6/nf_defrag_ipv6.h
+++ b/include/net/netfilter/ipv6/nf_defrag_ipv6.h
@@ -6,10 +6,7 @@ extern void nf_defrag_ipv6_enable(void);
extern int nf_ct_frag6_init(void);
extern void nf_ct_frag6_cleanup(void);
extern struct sk_buff *nf_ct_frag6_gather(struct sk_buff *skb, u32 user);
-extern void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
- struct net_device *in,
- struct net_device *out,
- int (*okfn)(struct sk_buff *));
+void nf_ct_frag6_consume_orig(struct sk_buff *skb);
struct inet_frags_ctl;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d81cff1..1371cf8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -580,9 +580,6 @@ static void skb_release_head_state(struct sk_buff *skb)
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
nf_conntrack_put(skb->nfct);
#endif
-#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
- nf_conntrack_put_reasm(skb->nfct_reasm);
-#endif
#ifdef CONFIG_BRIDGE_NETFILTER
nf_bridge_put(skb->nf_bridge);
#endif
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index d6e4dd8..83ab37c 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -169,63 +169,13 @@ out:
return nf_conntrack_confirm(skb);
}
-static unsigned int __ipv6_conntrack_in(struct net *net,
- unsigned int hooknum,
- struct sk_buff *skb,
- const struct net_device *in,
- const struct net_device *out,
- int (*okfn)(struct sk_buff *))
-{
- struct sk_buff *reasm = skb->nfct_reasm;
- const struct nf_conn_help *help;
- struct nf_conn *ct;
- enum ip_conntrack_info ctinfo;
-
- /* This packet is fragmented and has reassembled packet. */
- if (reasm) {
- /* Reassembled packet isn't parsed yet ? */
- if (!reasm->nfct) {
- unsigned int ret;
-
- ret = nf_conntrack_in(net, PF_INET6, hooknum, reasm);
- if (ret != NF_ACCEPT)
- return ret;
- }
-
- /* Conntrack helpers need the entire reassembled packet in the
- * POST_ROUTING hook. In case of unconfirmed connections NAT
- * might reassign a helper, so the entire packet is also
- * required.
- */
- ct = nf_ct_get(reasm, &ctinfo);
- if (ct != NULL && !nf_ct_is_untracked(ct)) {
- help = nfct_help(ct);
- if ((help && help->helper) || !nf_ct_is_confirmed(ct)) {
- nf_conntrack_get_reasm(reasm);
- NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, reasm,
- (struct net_device *)in,
- (struct net_device *)out,
- okfn, NF_IP6_PRI_CONNTRACK + 1);
- return NF_DROP_ERR(-ECANCELED);
- }
- }
-
- nf_conntrack_get(reasm->nfct);
- skb->nfct = reasm->nfct;
- skb->nfctinfo = reasm->nfctinfo;
- return NF_ACCEPT;
- }
-
- return nf_conntrack_in(net, PF_INET6, hooknum, skb);
-}
-
static unsigned int ipv6_conntrack_in(unsigned int hooknum,
struct sk_buff *skb,
const struct net_device *in,
const struct net_device *out,
int (*okfn)(struct sk_buff *))
{
- return __ipv6_conntrack_in(dev_net(in), hooknum, skb, in, out, okfn);
+ return nf_conntrack_in(dev_net(in), PF_INET6, hooknum, skb);
}
static unsigned int ipv6_conntrack_local(unsigned int hooknum,
@@ -239,7 +189,7 @@ static unsigned int ipv6_conntrack_local(unsigned int hooknum,
net_notice_ratelimited("ipv6_conntrack_local: packet too short\n");
return NF_ACCEPT;
}
- return __ipv6_conntrack_in(dev_net(out), hooknum, skb, in, out, okfn);
+ return nf_conntrack_in(dev_net(out), PF_INET6, hooknum, skb);
}
static struct nf_hook_ops ipv6_conntrack_ops[] __read_mostly = {
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index dffdc1a..253566a 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -621,31 +621,16 @@ ret_orig:
return skb;
}
-void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
- struct net_device *in, struct net_device *out,
- int (*okfn)(struct sk_buff *))
+void nf_ct_frag6_consume_orig(struct sk_buff *skb)
{
struct sk_buff *s, *s2;
- unsigned int ret = 0;
for (s = NFCT_FRAG6_CB(skb)->orig; s;) {
- nf_conntrack_put_reasm(s->nfct_reasm);
- nf_conntrack_get_reasm(skb);
- s->nfct_reasm = skb;
-
s2 = s->next;
s->next = NULL;
-
- if (ret != -ECANCELED)
- ret = NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, s,
- in, out, okfn,
- NF_IP6_PRI_CONNTRACK_DEFRAG + 1);
- else
- kfree_skb(s);
-
+ consume_skb(s);
s = s2;
}
- nf_conntrack_put_reasm(skb);
}
static int nf_ct_net_init(struct net *net)
diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
index aacd121..581dd9e 100644
--- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
+++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
@@ -75,8 +75,11 @@ static unsigned int ipv6_defrag(unsigned int hooknum,
if (reasm == skb)
return NF_ACCEPT;
- nf_ct_frag6_output(hooknum, reasm, (struct net_device *)in,
- (struct net_device *)out, okfn);
+ nf_ct_frag6_consume_orig(reasm);
+
+ NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, reasm,
+ (struct net_device *) in, (struct net_device *) out,
+ okfn, NF_IP6_PRI_CONNTRACK_DEFRAG + 1);
return NF_STOLEN;
}
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 74fd00c..3581736 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1139,12 +1139,6 @@ ip_vs_out(unsigned int hooknum, struct sk_buff *skb, int af)
ip_vs_fill_iph_skb(af, skb, &iph);
#ifdef CONFIG_IP_VS_IPV6
if (af == AF_INET6) {
- if (!iph.fragoffs && skb_nfct_reasm(skb)) {
- struct sk_buff *reasm = skb_nfct_reasm(skb);
- /* Save fw mark for coming frags */
- reasm->ipvs_property = 1;
- reasm->mark = skb->mark;
- }
if (unlikely(iph.protocol == IPPROTO_ICMPV6)) {
int related;
int verdict = ip_vs_out_icmp_v6(skb, &related,
@@ -1614,12 +1608,6 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
#ifdef CONFIG_IP_VS_IPV6
if (af == AF_INET6) {
- if (!iph.fragoffs && skb_nfct_reasm(skb)) {
- struct sk_buff *reasm = skb_nfct_reasm(skb);
- /* Save fw mark for coming frags. */
- reasm->ipvs_property = 1;
- reasm->mark = skb->mark;
- }
if (unlikely(iph.protocol == IPPROTO_ICMPV6)) {
int related;
int verdict = ip_vs_in_icmp_v6(skb, &related, hooknum,
@@ -1671,9 +1659,8 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
/* sorry, all this trouble for a no-hit :) */
IP_VS_DBG_PKT(12, af, pp, skb, 0,
"ip_vs_in: packet continues traversal as normal");
- if (iph.fragoffs && !skb_nfct_reasm(skb)) {
+ if (iph.fragoffs) {
/* Fragment that couldn't be mapped to a conn entry
- * and don't have any pointer to a reasm skb
* is missing module nf_defrag_ipv6
*/
IP_VS_DBG_RL("Unhandled frag, load nf_defrag_ipv6\n");
@@ -1756,38 +1743,6 @@ ip_vs_local_request4(unsigned int hooknum, struct sk_buff *skb,
#ifdef CONFIG_IP_VS_IPV6
/*
- * AF_INET6 fragment handling
- * Copy info from first fragment, to the rest of them.
- */
-static unsigned int
-ip_vs_preroute_frag6(unsigned int hooknum, struct sk_buff *skb,
- const struct net_device *in,
- const struct net_device *out,
- int (*okfn)(struct sk_buff *))
-{
- struct sk_buff *reasm = skb_nfct_reasm(skb);
- struct net *net;
-
- /* Skip if not a "replay" from nf_ct_frag6_output or first fragment.
- * ipvs_property is set when checking first fragment
- * in ip_vs_in() and ip_vs_out().
- */
- if (reasm)
- IP_VS_DBG(2, "Fragment recv prop:%d\n", reasm->ipvs_property);
- if (!reasm || !reasm->ipvs_property)
- return NF_ACCEPT;
-
- net = skb_net(skb);
- if (!net_ipvs(net)->enable)
- return NF_ACCEPT;
-
- /* Copy stored fw mark, saved in ip_vs_{in,out} */
- skb->mark = reasm->mark;
-
- return NF_ACCEPT;
-}
-
-/*
* AF_INET6 handler in NF_INET_LOCAL_IN chain
* Schedule and forward packets from remote clients
*/
@@ -1924,14 +1879,6 @@ static struct nf_hook_ops ip_vs_ops[] __read_mostly = {
.priority = 100,
},
#ifdef CONFIG_IP_VS_IPV6
- /* After mangle & nat fetch 2:nd fragment and following */
- {
- .hook = ip_vs_preroute_frag6,
- .owner = THIS_MODULE,
- .pf = NFPROTO_IPV6,
- .hooknum = NF_INET_PRE_ROUTING,
- .priority = NF_IP6_PRI_NAT_DST + 1,
- },
/* After packet filtering, change source only for VS/NAT */
{
.hook = ip_vs_reply6,
diff --git a/net/netfilter/ipvs/ip_vs_pe_sip.c b/net/netfilter/ipvs/ip_vs_pe_sip.c
index 9ef22bd..bed5f70 100644
--- a/net/netfilter/ipvs/ip_vs_pe_sip.c
+++ b/net/netfilter/ipvs/ip_vs_pe_sip.c
@@ -65,7 +65,6 @@ static int get_callid(const char *dptr, unsigned int dataoff,
static int
ip_vs_sip_fill_param(struct ip_vs_conn_param *p, struct sk_buff *skb)
{
- struct sk_buff *reasm = skb_nfct_reasm(skb);
struct ip_vs_iphdr iph;
unsigned int dataoff, datalen, matchoff, matchlen;
const char *dptr;
@@ -79,15 +78,10 @@ ip_vs_sip_fill_param(struct ip_vs_conn_param *p, struct sk_buff *skb)
/* todo: IPv6 fragments:
* I think this only should be done for the first fragment. /HS
*/
- if (reasm) {
- skb = reasm;
- dataoff = iph.thoff_reasm + sizeof(struct udphdr);
- } else
- dataoff = iph.len + sizeof(struct udphdr);
+ dataoff = iph.len + sizeof(struct udphdr);
if (dataoff >= skb->len)
return -EINVAL;
- /* todo: Check if this will mess-up the reasm skb !!! /HS */
retc = skb_linearize(skb);
if (retc < 0)
return retc;
--
1.8.4.3
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: Sarah Sharp @ 2013-11-19 22:58 UTC (permalink / raw)
To: David Laight; +Cc: Ben Hutchings, Alan Stern, netdev, linux-usb
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7430@saturn3.aculab.com>
On Mon, Nov 18, 2013 at 03:41:00PM -0000, David Laight wrote:
>
>
> > -----Original Message-----
> > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > Sent: 18 November 2013 15:03
> > To: David Laight
> > Cc: Alan Stern; Sarah Sharp; netdev@vger.kernel.org; linux-usb@vger.kernel.org
> > Subject: Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
> >
> > On Mon, 2013-11-18 at 09:48 +0000, David Laight wrote:
> > > > > But the minimum fragment size is (probably) 4k.
> > > > > For the network stack an OUT transfer might have a lot (and I mean lots)
> > > > > of fragments (there may be constraints, and linearising the skb is a option).
> > > > [...]
> > > >
> > > > The maximum number of fragments in the skb is going to be 17 (including
> > > > the 'head' area). (I'm ignoring NETIF_F_FRAGLIST which is not normally
> > > > supported by physical device drivers.)
> > > >
> > > > I don't know how many fragments that can end up as, at the USB level.
> > >
> > > If you assume that every fragment crosses a 64k boundary that would be 34.
> > > OTOH I've not seen a fragment of a 64k TSO send crossing a 32k
> > > boundary, and I think the 'head' area is constrained to be part of
> > > a single (4k or larger) page.
> >
> > I don't know that it's possible at the moment, but I wouldn't recommend
> > relying on that.
>
> The xhci (USB3) hardware supports SG, but a non-obvious alignment restriction
> applies at the end of a ring segment. In effect this means that the number of
> fragments mustn't exceed the size of the ring segment.
> It would make the xchi driver simpler if excessively fragmented requests
> could just be bounced.
> Since ring entries are 16 bytes there isn't much reason to not use a 4k
> 'page' for a ring and have (almost) 256 ring slots.
> (The code currently uses multiple ring segments with 63 usable slots.)
>
> Looks like skb are constrained enough that a sensible limit can be applied.
>
> The other likely generator of fragmented requests is the mass storage code.
> Most likely for dd(1) with a large block size.
The xHCI driver can limit the number of sg-list entries through
hcd->self.sg_tablesize. It's currently set to ~0, which is "however
many entries you want. You could set that to the number of TRBs in a
segment (minus one for the link TRB).
The usb-storage and uas drivers currently use sg_tablesize. Could the
network stack be taught to use sg_tablesize as well?
(Also, usb-storage aligns the block sizes to 512K, which explains why
we've never had an issue with TD fragments with that driver.)
Sarah Sharp
^ permalink raw reply
* [PATCH net] packet: fix use after free race in send path when dev is released
From: Daniel Borkmann @ 2013-11-19 23:08 UTC (permalink / raw)
To: davem; +Cc: netdev, Salam Noureddine, Ben Greear
Salam reported a use after free bug in PF_PACKET that occurs when
we're sending out frames on a socket bound device and suddenly the
netdevice is being unregistered. It appears that commit 827d9780
introduced a possible race condition between {t,}packet_snd and
packet_notifier. In the case of a bound socket, packet_notifier can
drop the last reference to the net_device and {t,}packet_snd might
end up sending a packet over a freed net_device.
To avoid reverting 827d9780 entirely, we could make use of po->running
member that gets reset when we're calling __unregister_prot_hook() in
packet_notifier() when we receive NETDEV_DOWN or NETDEV_UNREGISTER
notification. Plus, we still need to hold ref to the netdev, so
that we can assure it won't be released while we're in send path.
We try to avoid holding the bind lock all over send code as this
would likely make it worse when multiple threads want to send on
the same socket (in mmap code we're holding the pg_vec_lock mutex
all over the send code; we should try to get rid of that at some
point in time!). This at least buys us the possibility that we don't
have to walk over the entire dev list in dev_get_by_index() that we
would need to do as before 827d9780, which can just hurt as well
with many devices registered.
Fixes: 827d978037d7 ("af-packet: Use existing netdev reference for bound sockets.")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
Tested-by: Salam Noureddine <noureddine@aristanetworks.com>
Cc: Ben Greear <greearb@candelatech.com>
---
net/packet/af_packet.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..385b2cc 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1045,6 +1045,19 @@ static void *__prb_previous_block(struct packet_sock *po,
return prb_lookup_block(po, rb, previous, status);
}
+static struct net_device *dev_get_packet_sock(struct packet_sock *po)
+{
+ struct net_device *dev;
+
+ spin_lock(&po->bind_lock);
+ dev = po->prot_hook.dev;
+ if (dev)
+ dev_hold(dev);
+ spin_unlock(&po->bind_lock);
+
+ return dev;
+}
+
static void *packet_previous_rx_frame(struct packet_sock *po,
struct packet_ring_buffer *rb,
int status)
@@ -2057,7 +2070,6 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
struct sk_buff *skb;
struct net_device *dev;
__be16 proto;
- bool need_rls_dev = false;
int err, reserve = 0;
void *ph;
struct sockaddr_ll *saddr = (struct sockaddr_ll *)msg->msg_name;
@@ -2070,7 +2082,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
mutex_lock(&po->pg_vec_lock);
if (saddr == NULL) {
- dev = po->prot_hook.dev;
+ dev = dev_get_packet_sock(po);
proto = po->num;
addr = NULL;
} else {
@@ -2084,7 +2096,6 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
proto = saddr->sll_protocol;
addr = saddr->sll_addr;
dev = dev_get_by_index(sock_net(&po->sk), saddr->sll_ifindex);
- need_rls_dev = true;
}
err = -ENXIO;
@@ -2094,7 +2105,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
reserve = dev->hard_header_len;
err = -ENETDOWN;
- if (unlikely(!(dev->flags & IFF_UP)))
+ if (unlikely(!(dev->flags & IFF_UP) || !po->running))
goto out_put;
size_max = po->tx_ring.frame_size
@@ -2173,8 +2184,7 @@ out_status:
__packet_set_status(po, ph, status);
kfree_skb(skb);
out_put:
- if (need_rls_dev)
- dev_put(dev);
+ dev_put(dev);
out:
mutex_unlock(&po->pg_vec_lock);
return err;
@@ -2212,7 +2222,6 @@ static int packet_snd(struct socket *sock,
struct sk_buff *skb;
struct net_device *dev;
__be16 proto;
- bool need_rls_dev = false;
unsigned char *addr;
int err, reserve = 0;
struct virtio_net_hdr vnet_hdr = { 0 };
@@ -2228,7 +2237,7 @@ static int packet_snd(struct socket *sock,
*/
if (saddr == NULL) {
- dev = po->prot_hook.dev;
+ dev = dev_get_packet_sock(po);
proto = po->num;
addr = NULL;
} else {
@@ -2240,7 +2249,6 @@ static int packet_snd(struct socket *sock,
proto = saddr->sll_protocol;
addr = saddr->sll_addr;
dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex);
- need_rls_dev = true;
}
err = -ENXIO;
@@ -2250,7 +2258,7 @@ static int packet_snd(struct socket *sock,
reserve = dev->hard_header_len;
err = -ENETDOWN;
- if (!(dev->flags & IFF_UP))
+ if (unlikely(!(dev->flags & IFF_UP) || !po->running))
goto out_unlock;
if (po->has_vnet_hdr) {
@@ -2386,15 +2394,14 @@ static int packet_snd(struct socket *sock,
if (err > 0 && (err = net_xmit_errno(err)) != 0)
goto out_unlock;
- if (need_rls_dev)
- dev_put(dev);
+ dev_put(dev);
return len;
out_free:
kfree_skb(skb);
out_unlock:
- if (dev && need_rls_dev)
+ if (dev)
dev_put(dev);
out:
return err;
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH] rds: Error on offset mismatch if not loopback
From: Venkat Venkatsubra @ 2013-11-19 23:33 UTC (permalink / raw)
To: Honggang LI, Josh Hunt; +Cc: David Miller, jjolly, LKML, netdev
In-Reply-To: <528587D0.5060105@redhat.com>
We now have lot more information than we did before.
When sending a "congestion update" in rds_ib_xmit() we are now returning an incorrect number as bytes sent:
BUG_ON(off % RDS_FRAG_SIZE);
BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header));
/* Do not send cong updates to IB loopback */
if (conn->c_loopback
&& rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
scat = &rm->data.op_sg[sg];
ret = sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
ret = min_t(int, ret, scat->length - conn->c_xmit_data_off);
return ret;
}
It returns min(8240, 4096-0) i.e. 4096 bytes.
The caller rds_send_xmit() is made to think a partial message (4096 out of 8240) was sent.
It calls rds_ib_xmit() again with a data offset "off" of 4096-48 (rds header) (=4048 bytes). And we hit the BUG_ON.
The reason I didn't hit the panic on my test on Oracle UEK2 which is based on 2.6.39 kernel is it had it like this:
BUG_ON(off % RDS_FRAG_SIZE);
BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header));
/* Do not send cong updates to IB loopback */
if (conn->c_loopback
&& rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
return sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
}
(So it wasn't 100% 2.6.39 ;-). )
It returned 8240 bytes. The caller rds_send_xmit decides the full message was sent (48 byte header + 4096 data + 4096 data).
And it worked.
Then I found this info on the change that was done upstream which now causes the panic:
http://marc.info/?l=linux-netdev&m=129908332903057
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6094628bfd94323fc1cea05ec2c6affd98c18f7f
Will investigate more into which problem the above change addressed.
Venkat
^ permalink raw reply
* Re: [PATCH net] packet: fix use after free race in send path when dev is released
From: Eric Dumazet @ 2013-11-19 23:33 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, Salam Noureddine, Ben Greear
In-Reply-To: <1384902503-1588-1-git-send-email-dborkman@redhat.com>
On Wed, 2013-11-20 at 00:08 +0100, Daniel Borkmann wrote:
> Salam reported a use after free bug in PF_PACKET that occurs when
> we're sending out frames on a socket bound device and suddenly the
> netdevice is being unregistered. It appears that commit 827d9780
> introduced a possible race condition between {t,}packet_snd and
> packet_notifier. In the case of a bound socket, packet_notifier can
> drop the last reference to the net_device and {t,}packet_snd might
> end up sending a packet over a freed net_device.
>
> To avoid reverting 827d9780 entirely, we could make use of po->running
> member that gets reset when we're calling __unregister_prot_hook() in
> packet_notifier() when we receive NETDEV_DOWN or NETDEV_UNREGISTER
> notification. Plus, we still need to hold ref to the netdev, so
> that we can assure it won't be released while we're in send path.
> We try to avoid holding the bind lock all over send code as this
> would likely make it worse when multiple threads want to send on
> the same socket (in mmap code we're holding the pg_vec_lock mutex
> all over the send code; we should try to get rid of that at some
> point in time!). This at least buys us the possibility that we don't
> have to walk over the entire dev list in dev_get_by_index() that we
> would need to do as before 827d9780, which can just hurt as well
> with many devices registered.
>
> Fixes: 827d978037d7 ("af-packet: Use existing netdev reference for bound sockets.")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
> Tested-by: Salam Noureddine <noureddine@aristanetworks.com>
> Cc: Ben Greear <greearb@candelatech.com>
> ---
> net/packet/af_packet.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 2e8286b..385b2cc 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1045,6 +1045,19 @@ static void *__prb_previous_block(struct packet_sock *po,
> return prb_lookup_block(po, rb, previous, status);
> }
>
> +static struct net_device *dev_get_packet_sock(struct packet_sock *po)
> +{
> + struct net_device *dev;
> +
> + spin_lock(&po->bind_lock);
> + dev = po->prot_hook.dev;
> + if (dev)
> + dev_hold(dev);
> + spin_unlock(&po->bind_lock);
> +
> + return dev;
> +}
> +
...
dev_get_by_index() would be faster in fact.
Are you sure rcu_read_lock() is not possible ?
rcu_read_lock();
dev = rcu_dereference(po->active_device);
if (dev)
dev_hold(dev);
rcu_read_unlock();
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Arnaud Ebalard @ 2013-11-19 23:53 UTC (permalink / raw)
To: Willy Tarreau, Eric Dumazet
Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, netdev,
edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <20131119184121.GN913@1wt.eu>
Hi,
Willy Tarreau <w@1wt.eu> writes:
> On Tue, Nov 19, 2013 at 10:31:50AM -0800, Eric Dumazet wrote:
>> On Tue, 2013-11-19 at 18:43 +0100, Willy Tarreau wrote:
>>
>> > - #define MVNETA_TX_DONE_TIMER_PERIOD 10
>> > + #define MVNETA_TX_DONE_TIMER_PERIOD (1000/HZ)
>> >
>>
>> I suggested this in a prior mail :
>>
>> #define MVNETA_TX_DONE_TIMER_PERIOD 1
>
> Ah sorry, I remember now.
>
>> But apparently it was triggering strange crashes...
>
> Ah, when a bug hides another one, it's the situation I prefer, because
> by working on one, you end up fixing two :-)
Follow me just for one sec: today, I got a USB 3.0 Gigabit Ethernet
adapter. More specifically an AX88179-based one (Logitec LAN-GTJU3H3),
about which there is currently a thread on netdev and linux-usb
lists. Anyway, I decided to give it a try on my RN102 just to check what
performance I could achieve. So I basically did the same experiment as
yesterday (wget on client against a 1GB file located on the filesystem
served by an apache on the NAS) except that time the AX88179-based
adapater was used instead of the mvneta-based interface. Well, the
download started at a high rate (90MB/s) but then drops and I get some
SATA error on the NAS (similar to the errors I already got during
12.0-rc series [1] to finally *erroneously* consider it was an artefact).
So I decided to remove the SATA controllers and disks from the equation:
I switched to my ReadyNAS 2120 whose GbE interfaces are also based on
mvneta driver and comes w/ 2GB of RAM. The main additional difference is
that the device is a dual core armada @1.2GHz, where the RN102 is a
single core armada @1.2GHz. I created a dummy 1GB file *in RAM*
(/run/shm) to have it served by the apache2 instead of the file
previously stored on the disks.
I started w/ todays linus tree (dec8e46178b) with Eric's revert patch
for c9eeec26e32e (tcp: TSQ can use a dynamic limit) and also the change
to mvneta driver to have:
-#define MVNETA_TX_DONE_TIMER_PERIOD 10
+#define MVNETA_TX_DONE_TIMER_PERIOD 1
Here are the average speed given by wget for the following TCP send
window:
4 MB: 19 MB/s
2 MB: 21 MB/s
1 MB: 21 MB/s
512KB: 23 MB/s
384KB: 105 MB/s
256KB: 112 MB/s
128KB: 111 MB/s
64KB: 93 MB/s
Then, I decided to redo the exact same test w/o the change on
MVNETA_TX_DONE_TIMER_PERIOD (i.e. w/ the initial value of 10). I get the
exact same results as with the MVNETA_TX_DONE_TIMER_PERIOD set to 1, i.e:
4 MB: 20 MB/s
2 MB: 21 MB/s
1 MB: 21 MB/s
512KB: 22 MB/s
384KB: 105 MB/s
256KB: 112 MB/s
128KB: 111 MB/s
64KB: 93 MB/s
And, then, I also dropped Eric's revert patch for c9eeec26e32e (tcp: TSQ
can use a dynamic limit), just to verify we came back where the thread
started but i got a surprise:
4 MB: 10 MB/s
2 MB: 11 MB/s
1 MB: 10 MB/s
512KB: 12 MB/s
384KB: 104 MB/s
256KB: 112 MB/s
128KB: 112 MB/s
64KB: 93 MB/s
Instead of the 256KB/s I had observed the low value was now 10MB/s. I
thought it was due to the switch from RN102 to RN2120 so I came back
to the RN102 w/o any specific patch for mvneta nor your revert patch for
c9eeec26e32e, i.e. only Linus tree as it is today (dec8e46178b). The
file is served from the disk:
4 MB: 5 MB/s
2 MB: 5 MB/s
1 MB: 5 MB/s
512KB: 5 MB/s
384KB: 90 MB/s for 4s, then 3 MB/s
256KB: 80 MB/s for 3s, then 2 MB/s
128KB: 90 MB/s for 3s, then 3 MB/s
64KB: 80 MB/s for 3s, then 3 MB/S
Then, I allocated a dummy 400MB file in RAM (/run/shm) and redid the
test on the RN102:
4 MB: 8 MB/s
2 MB: 8 MB/s
1 MB: 92 MB/s
512KB: 90 MB/s
384KB: 90 MB/s
256KB: 90 MB/s
128KB: 90 MB/s
64KB: 60 MB/s
In the end, here are the conclusions *I* draw from this test session,
do not hesitate to correct me:
- Eric, it seems something changed in linus tree betwen the beginning
of the thread and now, which somehow reduces the effect of the
regression we were seen: I never got back the 256KB/s.
- You revert patch still improves the perf a lot
- It seems reducing MVNETA_TX_DONE_TIMER_PERIOD does not help
- w/ your revert patch, I can confirm that mvneta driver is capable of
doing line rate w/ proper tweak of TCP send window (256KB instead of
4M)
- It seems I will I have to spend some time on the SATA issues I
previously thought were an artefact of not cleaning my tree during a
debug session [1], i.e. there is IMHO an issue.
What I do not get is what can cause the perf to drop from 90MB/s to
3MB/s (w/ a 256KB send window) when streaming from the disk instead of
the RAM. I have no issue having dd read from the fs @ 150MB/s and
mvneta streaming from RAM @ 90MB/s but both together get me 3MB/s after
a few seconds.
Anyway, I think if the thread keeps going on improving mvneta, I'll do
all additional tests from RAM and will stop polluting netdev w/ possible
sata/disk/fs issues.
Cheers,
a+
[1]: http://thread.gmane.org/gmane.linux.ports.arm.kernel/271508
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-20 0:08 UTC (permalink / raw)
To: Arnaud Ebalard
Cc: Willy Tarreau, Thomas Petazzoni, Florian Fainelli, simon.guinot,
netdev, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <874n780wzc.fsf@natisbad.org>
On Wed, 2013-11-20 at 00:53 +0100, Arnaud Ebalard wrote:
> Anyway, I think if the thread keeps going on improving mvneta, I'll do
> all additional tests from RAM and will stop polluting netdev w/ possible
> sata/disk/fs issues.
;)
Alternative would be to use netperf or iperf to not use disk at all
and focus on TCP/network issues only.
Thanks !
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 0:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, netdev,
Arnaud Ebalard, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <1384906129.8604.132.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 04:08:49PM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 00:53 +0100, Arnaud Ebalard wrote:
>
> > Anyway, I think if the thread keeps going on improving mvneta, I'll do
> > all additional tests from RAM and will stop polluting netdev w/ possible
> > sata/disk/fs issues.
>
> ;)
>
> Alternative would be to use netperf or iperf to not use disk at all
> and focus on TCP/network issues only.
Yes, that's for the same reason that I continue to use inject/httpterm
for such purposes :
- httpterm uses tee()+splice() to send pre-built pages without copying ;
- inject uses recv(MSG_TRUNC) to ack everything without copying.
Both of them are really interesting to test the hardware's capabilities
and to push components in the middle to their limits without causing too
much burden to the end points.
I don't know if either netperf or iperf can make use of this now, and
I've been used to my tools, but I should take a look again.
Cheers,
Willy
^ permalink raw reply
* [PATCHv2] net: core: Always propagate flag changes to interfaces
From: Vlad Yasevich @ 2013-11-20 0:37 UTC (permalink / raw)
To: netdev; +Cc: s.priebe, vfalico, Vlad Yasevich
The following commit:
b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
net: only invoke dev->change_rx_flags when device is UP
tried to fix a problem with VLAN devices and promiscuouse flag setting.
The issue was that VLAN device was setting a flag on an interface that
was down, thus resulting in bad promiscuity count.
This commit blocked flag propagation to any device that is currently
down.
A later commit:
deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
vlan: Don't propagate flag changes on down interfaces
fixed VLAN code to only propagate flags when the VLAN interface is up,
thus fixing the same issue as above, only localized to VLAN.
The problem we have now is that if we have create a complex stack
involving multiple software devices like bridges, bonds, and vlans,
then it is possible that the flags would not propagate properly to
the physical devices. A simple examle of the scenario is the
following:
eth0----> bond0 ----> bridge0 ---> vlan50
If bond0 or eth0 happen to be down at the time bond0 is added to
the bridge, then eth0 will never have promisc mode set which is
currently required for operation as part of the bridge. As a
result, packets with vlan50 will be dropped by the interface.
The only 2 devices that implement the special flag handling are
VLAN and DSA and they both have required code to prevent incorrect
flag propagation. As a result we can remove the generic solution
introduced in b6c40d68ff6498b7f63ddf97cf0aa818d748dee7 and leave
it to the individual devices to decide whether they will block
flag propagation or not.
Reported-by: Stefan Priebe <s.priebe@profihost.ag>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
v1-v2: rebased on the proper net tree.
net/core/dev.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 974143d..d856e34 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4991,7 +4991,7 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
{
const struct net_device_ops *ops = dev->netdev_ops;
- if ((dev->flags & IFF_UP) && ops->ndo_change_rx_flags)
+ if (ops->ndo_change_rx_flags)
ops->ndo_change_rx_flags(dev, flags);
}
@@ -6885,10 +6885,6 @@ static struct pernet_operations __net_initdata default_device_ops = {
.exit = default_device_exit,
.exit_batch = default_device_exit_batch,
};
-
-/*
- * Initialize the DEV module. At boot time this walks the device list and
- * unhooks any devices that fail to initialise (normally hardware not
* present) and leaves us with a valid list of present and active devices.
*
*/
--
1.8.4.2
^ permalink raw reply related
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-20 0:43 UTC (permalink / raw)
To: Willy Tarreau
Cc: Arnaud Ebalard, Thomas Petazzoni, Florian Fainelli, simon.guinot,
netdev, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <20131120003519.GA22150@1wt.eu>
On Wed, 2013-11-20 at 01:35 +0100, Willy Tarreau wrote:
> On Tue, Nov 19, 2013 at 04:08:49PM -0800, Eric Dumazet wrote:
> > On Wed, 2013-11-20 at 00:53 +0100, Arnaud Ebalard wrote:
> >
> > > Anyway, I think if the thread keeps going on improving mvneta, I'll do
> > > all additional tests from RAM and will stop polluting netdev w/ possible
> > > sata/disk/fs issues.
> >
> > ;)
> >
> > Alternative would be to use netperf or iperf to not use disk at all
> > and focus on TCP/network issues only.
>
> Yes, that's for the same reason that I continue to use inject/httpterm
> for such purposes :
> - httpterm uses tee()+splice() to send pre-built pages without copying ;
> - inject uses recv(MSG_TRUNC) to ack everything without copying.
>
> Both of them are really interesting to test the hardware's capabilities
> and to push components in the middle to their limits without causing too
> much burden to the end points.
>
> I don't know if either netperf or iperf can make use of this now, and
> I've been used to my tools, but I should take a look again.
netperf -t TCP_SENDFILE does the zero copy at sender.
And more generally -V option does copy avoidance
(Use splice(sockfd -> nullfd) at receiver if I remember well)
Anyway, we should do the normal copy, because it might demonstrate
scheduling problems.
If you want to test raw speed, you could use pktgen ;)
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 0:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, netdev,
Arnaud Ebalard, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <1384908228.8604.136.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 04:43:48PM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 01:35 +0100, Willy Tarreau wrote:
> > On Tue, Nov 19, 2013 at 04:08:49PM -0800, Eric Dumazet wrote:
> > > On Wed, 2013-11-20 at 00:53 +0100, Arnaud Ebalard wrote:
> > >
> > > > Anyway, I think if the thread keeps going on improving mvneta, I'll do
> > > > all additional tests from RAM and will stop polluting netdev w/ possible
> > > > sata/disk/fs issues.
> > >
> > > ;)
> > >
> > > Alternative would be to use netperf or iperf to not use disk at all
> > > and focus on TCP/network issues only.
> >
> > Yes, that's for the same reason that I continue to use inject/httpterm
> > for such purposes :
> > - httpterm uses tee()+splice() to send pre-built pages without copying ;
> > - inject uses recv(MSG_TRUNC) to ack everything without copying.
> >
> > Both of them are really interesting to test the hardware's capabilities
> > and to push components in the middle to their limits without causing too
> > much burden to the end points.
> >
> > I don't know if either netperf or iperf can make use of this now, and
> > I've been used to my tools, but I should take a look again.
>
> netperf -t TCP_SENDFILE does the zero copy at sender.
>
> And more generally -V option does copy avoidance
>
> (Use splice(sockfd -> nullfd) at receiver if I remember well)
OK thanks for the info.
> Anyway, we should do the normal copy, because it might demonstrate
> scheduling problems.
Yes, especially in this case, though I got the issue with GSO only,
so it might vary as well.
> If you want to test raw speed, you could use pktgen ;)
Except I'm mostly focused on HTTP as you know. And for generating higher
packet rates than pktgen, I have an absolutely ugly patch that I'm a
bit ashamed of for mvneta to multiply the number of emitted descriptors
for a given skb by skb->sk->sk_mark (which I set using setsockopt), this
allows me to generate up to 1.488 Mpps on a USB-powered system, not that
bad in my opinion :-)
Willy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox