linux-stable.git/net/tipc, branch linux-3.13.y

tipc: fix spinlock recursion bug for failed subscriptions

2014-04-14T13:47:20+00:00

[ Upstream commit a5d0e7c037119484a7006b883618bfa87996cb41 ]

If a topology event subscription fails for any reason, such as out
of memory, max number reached or because we received an invalid
request the correct behavior is to terminate the subscribers
connection to the topology server. This is currently broken and
produces the following oops:

[27.953662] tipc: Subscription rejected, illegal request
[27.955329] BUG: spinlock recursion on CPU#1, kworker/u4:0/6
[27.957066]  lock: 0xffff88003c67f408, .magic: dead4ead, .owner: kworker/u4:0/6, .owner_cpu: 1
[27.958054] CPU: 1 PID: 6 Comm: kworker/u4:0 Not tainted 3.14.0-rc6+ #5
[27.960230] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[27.960874] Workqueue: tipc_rcv tipc_recv_work [tipc]
[27.961430]  ffff88003c67f408 ffff88003de27c18 ffffffff815c0207 ffff88003de1c050
[27.962292]  ffff88003de27c38 ffffffff815beec5 ffff88003c67f408 ffffffff817f0a8a
[27.963152]  ffff88003de27c58 ffffffff815beeeb ffff88003c67f408 ffffffffa0013520
[27.964023] Call Trace:
[27.964292]  [] dump_stack+0x45/0x56
[27.964874]  [] spin_dump+0x8c/0x91
[27.965420]  [] spin_bug+0x21/0x26
[27.965995]  [] do_raw_spin_lock+0x116/0x140
[27.966631]  [] _raw_spin_lock_bh+0x15/0x20
[27.967256]  [] subscr_conn_shutdown_event+0x20/0xa0 [tipc]
[27.968051]  [] tipc_close_conn+0xa4/0xb0 [tipc]
[27.968722]  [] tipc_conn_terminate+0x1a/0x30 [tipc]
[27.969436]  [] subscr_conn_msg_event+0x1f2/0x2f0 [tipc]
[27.970209]  [] tipc_receive_from_sock+0x90/0xf0 [tipc]
[27.970972]  [] tipc_recv_work+0x29/0x50 [tipc]
[27.971633]  [] process_one_work+0x165/0x3e0
[27.972267]  [] worker_thread+0x119/0x3a0
[27.972896]  [] ? manage_workers.isra.25+0x2a0/0x2a0
[27.973622]  [] kthread+0xdf/0x100
[27.974168]  [] ? kthread_create_on_node+0x1a0/0x1a0
[27.974893]  [] ret_from_fork+0x7c/0xb0
[27.975466]  [] ? kthread_create_on_node+0x1a0/0x1a0

The recursion occurs when subscr_terminate tries to grab the
subscriber lock, which is already taken by subscr_conn_msg_event.
We fix this by checking if the request to establish a new
subscription was successful, and if not we initiate termination of
the subscriber after we have released the subscriber lock.

Signed-off-by: Erik Hugne 
Reviewed-by: Jon Maloy 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

tipc: don't log disabled tasklet handler errors

2014-04-14T13:47:18+00:00

[ Upstream commit 2892505ea170094f982516bb38105eac45f274b1 ]

Failure to schedule a TIPC tasklet with tipc_k_signal because the
tasklet handler is disabled is not an error. It means TIPC is
currently in the process of shutting down. We remove the error
logging in this case.

Signed-off-by: Erik Hugne 
Reviewed-by: Jon Maloy 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

tipc: fix memory leak during module removal

2014-04-14T13:47:18+00:00

[ Upstream commit 1bb8dce57f4d15233688c68990852a10eb1cd79f ]

When the TIPC module is removed, the tasklet handler is disabled
before all other subsystems. This will cause lingering publications
in the name table because the node_down tasklets responsible to
clean up publications from an unreachable node will never run.
When the name table is shut down, these publications are detected
and an error message is logged:
tipc: nametbl_stop(): orphaned hash chain detected
This is actually a memory leak, introduced with commit
993b858e37b3120ee76d9957a901cca22312ffaa ("tipc: correct the order
of stopping services at rmmod")

Instead of just logging an error and leaking memory, we free
the orphaned entries during nametable shutdown.

Signed-off-by: Erik Hugne 
Reviewed-by: Jon Maloy 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

tipc: drop subscriber connection id invalidation

2014-04-14T13:47:18+00:00

[ Upstream commit edcc0511b5ee7235282a688cd604e3ae7f9e1fc9 ]

When a topology server subscriber is disconnected, the associated
connection id is set to zero. A check vs zero is then done in the
subscription timeout function to see if the subscriber have been
shut down. This is unnecessary, because all subscription timers
will be cancelled when a subscriber terminates. Setting the
connection id to zero is actually harmful because id zero is the
identity of the topology server listening socket, and can cause a
race that leads to this socket being closed instead.

Signed-off-by: Erik Hugne 
Acked-by: Ying Xue 
Reviewed-by: Jon Maloy 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

tipc: fix connection refcount leak

2014-04-14T13:47:18+00:00

[ Upstream commit 4652edb70e8a7eebbe47fa931940f65522c36e8f ]

When tipc_conn_sendmsg() calls tipc_conn_lookup() to query a
connection instance, its reference count value is increased if
it's found. But subsequently if it's found that the connection is
closed, the work of sending message is not queued into its server
send workqueue, and the connection reference count is not decreased.
This will cause a reference count leak. To reproduce this problem,
an application would need to open and closes topology server
connections with high intensity.

We fix this by immediately decrementing the connection reference
count if a send fails due to the connection being closed.

Signed-off-by: Ying Xue 
Acked-by: Erik Hugne 
Reviewed-by: Jon Maloy 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

tipc: allow connection shutdown callback to be invoked in advance

2014-04-14T13:47:17+00:00

[ Upstream commit 6d4ebeb4df0176b1973875840a9f7e91394c0685 ]

Currently connection shutdown callback function is called when
connection instance is released in tipc_conn_kref_release(), and
receiving packets and sending packets are running in different
threads. Even if connection is closed by the thread of receiving
packets, its shutdown callback may not be called immediately as
the connection reference count is non-zero at that moment. So,
although the connection is shut down by the thread of receiving
packets, the thread of sending packets doesn't know it. Before
its shutdown callback is invoked to tell the sending thread its
connection has been closed, the sending thread may deliver
messages by tipc_conn_sendmsg(), this is why the following error
information appears:

"Sending subscription event failed, no memory"

To eliminate it, allow connection shutdown callback function to
be called before connection id is removed in tipc_close_conn(),
which makes the sending thread know the truth in time that its
socket is closed so that it doesn't send message to it. We also
remove the "Sending XXX failed..." error reporting for topology
and config services.

Signed-off-by: Ying Xue 
Signed-off-by: Erik Hugne 
Reviewed-by: Jon Maloy 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

tipc: correctly unlink packets from deferred packet queue

2014-01-07T21:15:24+00:00

When we pull a received packet from a link's 'deferred packets' queue
for processing, its 'next' pointer is not cleared, and still refers to
the next packet in that queue, if any. This is incorrect, but caused
no harm before commit 40ba3cdf542a469aaa9083fa041656e59b109b90 ("tipc:
message reassembly using fragment chain") was introduced. After that
commit, it may sometimes lead to the following oops:

general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
Modules linked in: tipc
CPU: 4 PID: 0 Comm: swapper/4 Tainted: G        W 3.13.0-rc2+ #6
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
task: ffff880017af4880 ti: ffff880017aee000 task.ti: ffff880017aee000
RIP: 0010:[]  [] skb_try_coalesce+0x44/0x3d0
RSP: 0018:ffff880016603a78  EFLAGS: 00010212
RAX: 6b6b6b6bd6d6d6d6 RBX: ffff880013106ac0 RCX: ffff880016603ad0
RDX: ffff880016603ad7 RSI: ffff88001223ed00 RDI: ffff880013106ac0
RBP: ffff880016603ab8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff88001223ed00
R13: ffff880016603ad0 R14: 000000000000058c R15: ffff880012297650
FS:  0000000000000000(0000) GS:ffff880016600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 000000000805b000 CR3: 0000000011f5d000 CR4: 00000000000006e0
Stack:
 ffff880016603a88 ffffffff810a38ed ffff880016603aa8 ffff88001223ed00
 0000000000000001 ffff880012297648 ffff880016603b68 ffff880012297650
 ffff880016603b08 ffffffffa0006c51 ffff880016603b08 00ffffffa00005fc
Call Trace:
 
 [] ? trace_hardirqs_on+0xd/0x10
 [] tipc_link_recv_fragment+0xd1/0x1b0 [tipc]
 [] tipc_recv_msg+0x4e4/0x920 [tipc]
 [] ? tipc_l2_rcv_msg+0x40/0x250 [tipc]
 [] tipc_l2_rcv_msg+0xcc/0x250 [tipc]
 [] ? tipc_l2_rcv_msg+0x40/0x250 [tipc]
 [] __netif_receive_skb_core+0x80b/0xd00
 [] ? __netif_receive_skb_core+0x144/0xd00
 [] __netif_receive_skb+0x26/0x70
 [] netif_receive_skb+0x2d/0x200
 [] napi_gro_receive+0xb0/0x130
 [] e1000_clean_rx_irq+0x2c2/0x530
 [] e1000_clean+0x266/0x9c0
 [] ? notifier_call_chain+0x2b/0x160
 [] net_rx_action+0x141/0x310
 [] __do_softirq+0xeb/0x480
 [] ? _raw_spin_unlock+0x2b/0x40
 [] ? handle_fasteoi_irq+0x72/0x100
 [] irq_exit+0x96/0xc0
 [] do_IRQ+0x63/0xe0
 [] common_interrupt+0x6f/0x6f
 

This happens when the last fragment of a message has passed through the
the receiving link's 'deferred packets' queue, and at least one other
packet was added to that queue while it was there. After the fragment
chain with the complete message has been successfully delivered to the
receiving socket, it is released. Since 'next' pointer of the last
fragment in the released chain now is non-NULL, we get the crash shown
above.

We fix this by clearing the 'next' pointer of all received packets,
including those being pulled from the 'deferred' queue, before they
undergo any further processing.

Fixes: 40ba3cdf542a4 ("tipc: message reassembly using fragment chain")
Signed-off-by: Erik Hugne 
Reported-by: Ying Xue 
Reviewed-by: Paul Gortmaker 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: fix deadlock during socket release

2013-12-30T03:24:07+00:00

A deadlock might occur if name table is withdrawn in socket release
routine, and while packets are still being received from bearer.

       CPU0                       CPU1
T0:   recv_msg()               release()
T1:   tipc_recv_msg()          tipc_withdraw()
T2:   [grab node lock]         [grab port lock]
T3:   tipc_link_wakeup_ports() tipc_nametbl_withdraw()
T4:   [grab port lock]*        named_cluster_distribute()
T5:   wakeupdispatch()         tipc_link_send()
T6:                            [grab node lock]*

The opposite order of holding port lock and node lock on above two
different paths may result in a deadlock. If socket lock instead of
port lock is used to protect port instance in tipc_withdraw(), the
reverse order of holding port lock and node lock will be eliminated,
as a result, the deadlock is killed as well.

Reported-by: Lars Everbrand 
Reviewed-by: Erik Hugne 
Signed-off-by: Ying Xue 
Signed-off-by: David S. Miller

tipc: protect handler_enabled variable with qitem_lock spin lock

2013-12-11T03:35:49+00:00

'handler_enabled' is a global flag indicating whether the TIPC
signal handling service is enabled or not. The lack of lock
protection for this flag incurs a risk for contention, so that
a tipc_k_signal() call might queue a signal handler to a destroyed
signal queue, with unpredictable results. To correct this, we let
the already existing 'qitem_lock' protect the flag, as it already
does with the queue itself. This way, we ensure that the flag
always is consistent across all cores.

Signed-off-by: Ying Xue 
Reviewed-by: Paul Gortmaker 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: correct the order of stopping services at rmmod

2013-12-11T03:35:49+00:00

The 'signal handler' service in TIPC is a mechanism that makes it
possible to postpone execution of functions, by launcing them into
a job queue for execution in a separate tasklet, independent of
the launching execution thread.

When we do rmmod on the tipc module, this service is stopped after
the network service. At the same time, the stopping of the network
service may itself launch jobs for execution, with the risk that these
functions may be scheduled for execution after the data structures
meant to be accessed by the job have already been deleted. We have
seen this happen, most often resulting in an oops.

This commit ensures that the signal handler is the very first to be
stopped when TIPC is shut down, so there are no surprises during
the cleanup of the other services.

Signed-off-by: Jon Maloy 
Reviewed-by: Paul Gortmaker 
Signed-off-by: David S. Miller