lwn.git/net/tipc/subscr.c, branch docs-for-linus

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

tipc: fix connection abort during subscription cancellation

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

In 'commit 7fe8097cef5f ("tipc: fix nullpointer bug when subscribing to events")', we terminate the connection if the subscription creation fails. In the same commit, the subscription creation result was based on the value of subscription pointer (set in the function) instead of the return code. Unfortunately, the same function also handles subscription cancellation request. For a subscription cancellation request, the subscription pointer cannot be set. Thus the connection is terminated during cancellation request. In this commit, we move the subcription cancel check outside of tipc_subscrp_create(). Hence, - tipc_subscrp_create() will create a subscripton - tipc_subscrb_rcv_cb() will subscribe or cancel a subscription. Fixes: 'commit 7fe8097cef5f ("tipc: fix nullpointer bug when subscribing to events")' Acked-by: Ying Xue Reviewed-by: Jon Maloy Signed-off-by: Parthasarathy Bhuvaragan Signed-off-by: David S. Miller

tipc: introduce tipc_subscrb_subscribe() routine

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

In this commit, we split tipc_subscrp_create() into two: 1. tipc_subscrp_create() creates a subscription 2. A new function tipc_subscrp_subscribe() adds the subscription to the subscriber subscription list, activates the subscription timer and subscribes to the nametable updates. In future commits, the purpose of tipc_subscrb_rcv_cb() will be to either subscribe or cancel a subscription. There is no functional change in this commit. Acked-by: Ying Xue Reviewed-by: Jon Maloy Signed-off-by: Parthasarathy Bhuvaragan Signed-off-by: David S. Miller