> -----Original Message-----
> From: Ying Xue [mailto:[email protected]]
> Sent: Monday, December 04, 2017 09:11
> To: [email protected]; Jon Maloy <[email protected]>
> Cc: [email protected]
> Subject: [net-next PATCH 1/2] tipc: prevent swork item from queuing
> workqueue after connection is closed
>
> For one same connection, tipc_close_conn() and tipc_conn_sendmsg() might
> be conducted on different cores in parallel. Once connection is flagged as
> close, we should not queue swork item to workqueue.
>
> But as sk->sk_callback_lock is not taken in tipc_conn_sendmsg(), a swork
> item might be queued into s->send_wq workqueue even if the connection
> has been shut down in tipc_close_conn() on another core.
>
> To prevent the race condition from happening, we first hold
> sk->sk_callback and then ensure connection flags and sk->sk_user_data
> are both valid before swork item is queued into s->send_wq workqueue.
>
> Signed-off-by: Ying Xue <[email protected]>
> ---
> net/tipc/server.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/net/tipc/server.c b/net/tipc/server.c index acaef80..571e137
> 100644
> --- a/net/tipc/server.c
> +++ b/net/tipc/server.c
> @@ -449,16 +449,12 @@ int tipc_conn_sendmsg(struct tipc_server *s, int
> conid, {
> struct outqueue_entry *e;
> struct tipc_conn *con;
> + struct sock *sk;
>
> con = tipc_conn_lookup(s, conid);
> if (!con)
> return -EINVAL;
>
> - if (!test_bit(CF_CONNECTED, &con->flags)) {
> - conn_put(con);
> - return 0;
> - }
> -
> e = tipc_alloc_entry(data, len);
> if (!e) {
> conn_put(con);
> @@ -472,8 +468,17 @@ int tipc_conn_sendmsg(struct tipc_server *s, int
> conid,
You cannot be sure that con is valid here. You need to add the
!kref_get_unless_zero(&con->kref)) in the lookup function, as I had to do.
This was caused by observed crashes.
> list_add_tail(&e->list, &con->outqueue);
> spin_unlock_bh(&con->outqueue_lock);
>
> - if (!queue_work(s->send_wq, &con->swork))
> + sk = con->sock->sk;
> +
You don't know if the socket still exists here. We may be in the middle of a
tipc_conn_close().
Furthermore, in the case of a kernel subscriber, there *is* no socket.
But I am not sure this patch is needed at all ??
///jon
> + read_lock_bh(&sk->sk_callback_lock);
> + if (sk->sk_user_data && test_bit(CF_CONNECTED, &con->flags)) {
> + if (!queue_work(s->send_wq, &con->swork))
> + conn_put(con);
> + } else {
> conn_put(con);
> + }
> + read_unlock_bh(&sk->sk_callback_lock);
> +
> return 0;
> }
>
> --
> 2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
