linux-toradex.git/net/tipc/subscr.c, branch v4.9.97

tipc: fix nametbl deadlock at tipc_nametbl_unsubscribe

2017-12-25T13:23:39+00:00

[ Upstream commit 557d054c01da0337ca81de9e9d9206d57245b57e ]

Until now, tipc_nametbl_unsubscribe() is called at subscriptions
reference count cleanup. Usually the subscriptions cleanup is
called at subscription timeout or at subscription cancel or at
subscriber delete.

We have ignored the possibility of this being called from other
locations, which causes deadlock as we try to grab the
tn->nametbl_lock while holding it already.

   CPU1:                             CPU2:
----------                     ----------------
tipc_nametbl_publish
spin_lock_bh(&tn->nametbl_lock)
tipc_nametbl_insert_publ
tipc_nameseq_insert_publ
tipc_subscrp_report_overlap
tipc_subscrp_get
tipc_subscrp_send_event
                             tipc_close_conn
                             tipc_subscrb_release_cb
                             tipc_subscrb_delete
                             tipc_subscrp_put
tipc_subscrp_put
tipc_subscrp_kref_release
tipc_nametbl_unsubscribe
spin_lock_bh(&tn->nametbl_lock)
<>

   CPU1:                              CPU2:
----------                     ----------------
tipc_nametbl_stop
spin_lock_bh(&tn->nametbl_lock)
tipc_purge_publications
tipc_nameseq_remove_publ
tipc_subscrp_report_overlap
tipc_subscrp_get
tipc_subscrp_send_event
                             tipc_close_conn
                             tipc_subscrb_release_cb
                             tipc_subscrb_delete
                             tipc_subscrp_put
tipc_subscrp_put
tipc_subscrp_kref_release
tipc_nametbl_unsubscribe
spin_lock_bh(&tn->nametbl_lock)
<>

In this commit, we advance the calling of tipc_nametbl_unsubscribe()
from the refcount cleanup to the intended callers.

Fixes: d094c4d5f5c7 ("tipc: add subscription refcount to avoid invalid delete")
Reported-by: John Thompson 
Acked-by: Jon Maloy 
Signed-off-by: Ying Xue 
Signed-off-by: Parthasarathy Bhuvaragan 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

tipc: add subscription refcount to avoid invalid delete

2017-06-17T04:41:57+00:00

[ Upstream commit d094c4d5f5c7e1b225e94227ca3f007be3adc4e8 ]

Until now, the subscribers keep track of the subscriptions using
reference count at subscriber level. At subscription cancel or
subscriber delete, we delete the subscription only if the timer
was pending for the subscription. This approach is incorrect as:
1. del_timer() is not SMP safe, if on CPU0 the check for pending
   timer returns true but CPU1 might schedule the timer callback
   thereby deleting the subscription. Thus when CPU0 is scheduled,
   it deletes an invalid subscription.
2. We export tipc_subscrp_report_overlap(), which accesses the
   subscription pointer multiple times. Meanwhile the subscription
   timer can expire thereby freeing the subscription and we might
   continue to access the subscription pointer leading to memory
   violations.

In this commit, we introduce subscription refcount to avoid deleting
an invalid subscription.

Reported-and-Tested-by: John Thompson 
Acked-by: Ying Xue 
Acked-by: Jon Maloy 
Signed-off-by: Parthasarathy Bhuvaragan 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

tipc: remove an unnecessary NULL check

2016-04-28T20:54:12+00:00

This is never called with a NULL "buf" and anyway, we dereference 's' on
the lines before so it would Oops before we reach the check.

Signed-off-by: Dan Carpenter 
Acked-by: Ying Xue 
Signed-off-by: David S. Miller

tipc: fix a race condition leading to subscriber refcnt bug

2016-04-14T20:46:46+00:00

Until now, the requests sent to topology server are queued
to a workqueue by the generic server framework.
These messages are processed by worker threads and trigger the
registered callbacks.
To reduce latency on uniprocessor systems, explicit rescheduling
is performed using cond_resched() after MAX_RECV_MSG_COUNT(25)
messages.

This implementation on SMP systems leads to an subscriber refcnt
error as described below:
When a worker thread yields by calling cond_resched() in a SMP
system, a new worker is created on another CPU to process the
pending workitem. Sometimes the sleeping thread wakes up before
the new thread finishes execution.
This breaks the assumption on ordering and being single threaded.
The fault is more frequent when MAX_RECV_MSG_COUNT is lowered.

If the first thread was processing subscription create and the
second thread processing close(), the close request will free
the subscriber and the create request oops as follows:

[31.224137] WARNING: CPU: 2 PID: 266 at include/linux/kref.h:46 tipc_subscrb_rcv_cb+0x317/0x380         [tipc]
[31.228143] CPU: 2 PID: 266 Comm: kworker/u8:1 Not tainted 4.5.0+ #97
[31.228377] Workqueue: tipc_rcv tipc_recv_work [tipc]
[...]
[31.228377] Call Trace:
[31.228377]  [] dump_stack+0x4d/0x72
[31.228377]  [] __warn+0xd1/0xf0
[31.228377]  [] warn_slowpath_null+0x1d/0x20
[31.228377]  [] tipc_subscrb_rcv_cb+0x317/0x380 [tipc]
[31.228377]  [] tipc_receive_from_sock+0xd4/0x130 [tipc]
[31.228377]  [] tipc_recv_work+0x2b/0x50 [tipc]
[31.228377]  [] process_one_work+0x145/0x3d0
[31.246554] ---[ end trace c3882c9baa05a4fd ]---
[31.248327] BUG: spinlock bad magic on CPU#2, kworker/u8:1/266
[31.249119] BUG: unable to handle kernel NULL pointer dereference at 0000000000000428
[31.249323] IP: [] spin_dump+0x5c/0xe0
[31.249323] PGD 0
[31.249323] Oops: 0000 [#1] SMP

In this commit, we
- rename tipc_conn_shutdown() to tipc_conn_release().
- move connection release callback execution from tipc_close_conn()
  to a new function tipc_sock_release(), which is executed before
  we free the connection.
Thus we release the subscriber during connection release procedure
rather than connection shutdown procedure.

Signed-off-by: Parthasarathy Bhuvaragan 
Acked-by: Ying Xue 
Signed-off-by: David S. Miller

Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net

2016-03-08T17:34:12+00:00

Several cases of overlapping changes, as well as one instance
(vxlan) of a bug fix in 'net' overlapping with code movement
in 'net-next'.

Signed-off-by: David S. Miller

tipc: fix nullptr crash during subscription cancel

2016-03-07T04:00:08+00:00

commit 4d5cfcba2f6e ('tipc: fix connection abort during subscription
cancel'), removes the check for a valid subscription before calling
tipc_nametbl_subscribe().

This will lead to a nullptr exception when we process a
subscription cancel request. For a cancel request, a null
subscription is passed to tipc_nametbl_subscribe() resulting
in exception.

In this commit, we call tipc_nametbl_subscribe() only for
a valid subscription.

Fixes: 4d5cfcba2f6e ('tipc: fix connection abort during subscription cancel')
Reported-by: Anders Widell 
Signed-off-by: Parthasarathy Bhuvaragan 
Acked-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: donot create timers if subscription timeout = TIPC_WAIT_FOREVER

2016-02-06T08:41:58+00:00

Until now, we create timers even for the subscription requests
with timeout = TIPC_WAIT_FOREVER.
This can be improved by avoiding timer creation when the timeout
is set to TIPC_WAIT_FOREVER.

In this commit, we introduce a check to creates timers only
when timeout != TIPC_WAIT_FOREVER.

Acked-by: Ying Xue 
Reviewed-by: Jon Maloy 
Signed-off-by: Parthasarathy Bhuvaragan 
Signed-off-by: David S. Miller

tipc: protect tipc_subscrb_get() with subscriber spin lock

2016-02-06T08:41:58+00:00

Until now, during subscription creation the mod_time() &
tipc_subscrb_get() are called after releasing the subscriber
spin lock.

In a SMP system when performing a subscription creation, if the
subscription timeout occurs simultaneously (the timer is
scheduled to run on another CPU) then the timer thread
might decrement the subscribers refcount before the create
thread increments the refcount.

This can be simulated by creating subscription with timeout=0 and
sometimes the timeout occurs before the create request is complete.
This leads to the following message:
[30.702949] BUG: spinlock bad magic on CPU#1, kworker/u8:3/87
[30.703834] general protection fault: 0000 [#1] SMP
[30.704826] CPU: 1 PID: 87 Comm: kworker/u8:3 Not tainted 4.4.0-rc8+ #18
[30.704826] Workqueue: tipc_rcv tipc_recv_work [tipc]
[30.704826] task: ffff88003f878600 ti: ffff88003fae0000 task.ti: ffff88003fae0000
[30.704826] RIP: 0010:[]  [] spin_dump+0x5c/0xe0
[...]
[30.704826] Call Trace:
[30.704826]  [] spin_bug+0x26/0x30
[30.704826]  [] do_raw_spin_lock+0xe5/0x120
[30.704826]  [] _raw_spin_lock_bh+0x19/0x20
[30.704826]  [] tipc_subscrb_rcv_cb+0x1d0/0x330 [tipc]
[30.704826]  [] tipc_receive_from_sock+0xc1/0x150 [tipc]
[30.704826]  [] tipc_recv_work+0x3f/0x80 [tipc]
[30.704826]  [] process_one_work+0x149/0x3c0
[30.704826]  [] worker_thread+0x66/0x460
[30.704826]  [] ? process_one_work+0x3c0/0x3c0
[30.704826]  [] ? process_one_work+0x3c0/0x3c0
[30.704826]  [] kthread+0xed/0x110
[30.704826]  [] ? kthread_create_on_node+0x190/0x190
[30.704826]  [] ret_from_fork+0x3f/0x70

In this commit,
1. we remove the check for the return code for mod_timer()
2. we protect tipc_subscrb_get() using the subscriber spin lock.
   We increment the subscriber's refcount as soon as we add the
   subscription to subscriber's subscription list.

Acked-by: Ying Xue 
Reviewed-by: Jon Maloy 
Signed-off-by: Parthasarathy Bhuvaragan 
Signed-off-by: David S. Miller

tipc: hold subscriber->lock for tipc_nametbl_subscribe()

2016-02-06T08:41:58+00:00

Until now, while creating a subscription the subscriber lock
protects only the subscribers subscription list and not the
nametable. The call to tipc_nametbl_subscribe() is outside
the lock. However, at subscription timeout and cancel both
the subscribers subscription list and the nametable are
protected by the subscriber lock.

This asymmetric locking mechanism leads to the following problem:
In a SMP system, the timer can be fire on another core before
the create request is complete.
When the timer thread calls tipc_nametbl_unsubscribe() before create
thread calls tipc_nametbl_subscribe(), we get a nullptr exception.

This can be simulated by creating subscription with timeout=0 and
sometimes the timeout occurs before the create request is complete.

The following is the oops:
[57.569661] BUG: unable to handle kernel NULL pointer dereference at (null)
[57.577498] IP: [] tipc_nametbl_unsubscribe+0x8a/0x120 [tipc]
[57.584820] PGD 0
[57.586834] Oops: 0002 [#1] SMP
[57.685506] CPU: 14 PID: 10077 Comm: kworker/u40:1 Tainted: P OENX 3.12.48-52.27.1.     9688.1.PTF-default #1
[57.703637] Workqueue: tipc_rcv tipc_recv_work [tipc]
[57.708697] task: ffff88064c7f00c0 ti: ffff880629ef4000 task.ti: ffff880629ef4000
[57.716181] RIP: 0010:[]  [] tipc_nametbl_unsubscribe+0x8a/   0x120 [tipc]
[...]
[57.812327] Call Trace:
[57.814806]  [] tipc_subscrp_delete+0x37/0x90 [tipc]
[57.821357]  [] tipc_subscrp_timeout+0x3f/0x70 [tipc]
[57.827982]  [] call_timer_fn+0x31/0x100
[57.833490]  [] run_timer_softirq+0x1f9/0x2b0
[57.839414]  [] __do_softirq+0xe5/0x230
[57.844827]  [] call_softirq+0x1c/0x30
[57.850150]  [] do_softirq+0x55/0x90
[57.855285]  [] irq_exit+0x95/0xa0
[57.860290]  [] smp_apic_timer_interrupt+0x45/0x60
[57.866644]  [] apic_timer_interrupt+0x6d/0x80
[57.872686]  [] tipc_subscrb_rcv_cb+0x2a5/0x3f0 [tipc]
[57.879425]  [] tipc_receive_from_sock+0x9f/0x100 [tipc]
[57.886324]  [] tipc_recv_work+0x26/0x60 [tipc]
[57.892463]  [] process_one_work+0x172/0x420
[57.898309]  [] worker_thread+0x11a/0x3c0
[57.903871]  [] kthread+0xb4/0xc0
[57.908751]  [] ret_from_fork+0x58/0x90

In this commit, we do the following at subscription creation:
1. set the subscription's subscriber pointer before performing
   tipc_nametbl_subscribe(), as this value is required further in
   the call chain ex: by tipc_subscrp_send_event().
2. move tipc_nametbl_subscribe() under the scope of subscriber lock

Acked-by: Ying Xue 
Reviewed-by: Jon Maloy 
Signed-off-by: Parthasarathy Bhuvaragan 
Signed-off-by: David S. Miller

tipc: fix connection abort when receiving invalid cancel request

2016-02-06T08:41:58+00:00

Until now, the subscribers endianness for a subscription
create/cancel request is determined as:
    swap = !(s->filter & (TIPC_SUB_PORTS | TIPC_SUB_SERVICE))
The checks are performed only for port/service subscriptions.

The swap calculation is incorrect if the filter in the subscription
cancellation request is set to TIPC_SUB_CANCEL (it's a malformed
cancel request, as the corresponding subscription create filter
is missing).
Thus, the check if the request is for cancellation fails and the
request is treated as a subscription create request. The
subscription creation fails as the request is illegal, which
terminates this connection.

In this commit we determine the endianness by including
TIPC_SUB_CANCEL, which will set swap correctly and the
request is processed as a cancellation request.

Acked-by: Ying Xue 
Reviewed-by: Jon Maloy 
Signed-off-by: Parthasarathy Bhuvaragan 
Signed-off-by: David S. Miller