From 3735b4b9f1c102dcaf70241225339bdea4447dc8 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 23 Sep 2016 14:23:26 -0400 Subject: dlm: don't save callbacks after accept When DLM calls accept() on a socket, the comm code copies the sk after we've saved its callbacks. Afterward, it calls add_sock which saves the callbacks a second time. Since the error reporting function lowcomms_error_report calls the previous callback too, this results in a recursive call to itself. This patch adds a new parameter to function add_sock to tell whether to save the callbacks. Function tcp_accept_from_sock (and its sctp counterpart) then calls it with false to avoid the recursion. Signed-off-by: Bob Peterson Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'fs/dlm/lowcomms.c') diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 609998de533e..485494e5a28e 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -541,7 +541,7 @@ static void restore_callbacks(struct connection *con, struct sock *sk) } /* Make a socket active */ -static void add_sock(struct socket *sock, struct connection *con) +static void add_sock(struct socket *sock, struct connection *con, bool save_cb) { struct sock *sk = sock->sk; @@ -549,7 +549,7 @@ static void add_sock(struct socket *sock, struct connection *con) con->sock = sock; sk->sk_user_data = con; - if (!test_bit(CF_IS_OTHERCON, &con->flags)) + if (save_cb) save_callbacks(con, sk); /* Install a data_ready callback */ sk->sk_data_ready = lowcomms_data_ready; @@ -806,7 +806,7 @@ static int tcp_accept_from_sock(struct connection *con) newcon->othercon = othercon; othercon->sock = newsock; newsock->sk->sk_user_data = othercon; - add_sock(newsock, othercon); + add_sock(newsock, othercon, false); addcon = othercon; } else { @@ -819,7 +819,10 @@ static int tcp_accept_from_sock(struct connection *con) else { newsock->sk->sk_user_data = newcon; newcon->rx_action = receive_from_sock; - add_sock(newsock, newcon); + /* accept copies the sk after we've saved the callbacks, so we + don't want to save them a second time or comm errors will + result in calling sk_error_report recursively. */ + add_sock(newsock, newcon, false); addcon = newcon; } @@ -919,7 +922,7 @@ static int sctp_accept_from_sock(struct connection *con) newcon->othercon = othercon; othercon->sock = newsock; newsock->sk->sk_user_data = othercon; - add_sock(newsock, othercon); + add_sock(newsock, othercon, false); addcon = othercon; } else { printk("Extra connection from node %d attempted\n", nodeid); @@ -930,7 +933,7 @@ static int sctp_accept_from_sock(struct connection *con) } else { newsock->sk->sk_user_data = newcon; newcon->rx_action = receive_from_sock; - add_sock(newsock, newcon); + add_sock(newsock, newcon, false); addcon = newcon; } @@ -1058,7 +1061,7 @@ static void sctp_connect_to_sock(struct connection *con) sock->sk->sk_user_data = con; con->rx_action = receive_from_sock; con->connect_action = sctp_connect_to_sock; - add_sock(sock, con); + add_sock(sock, con, true); /* Bind to all addresses. */ if (sctp_bind_addrs(con, 0)) @@ -1146,7 +1149,7 @@ static void tcp_connect_to_sock(struct connection *con) sock->sk->sk_user_data = con; con->rx_action = receive_from_sock; con->connect_action = tcp_connect_to_sock; - add_sock(sock, con); + add_sock(sock, con, true); /* Bind to our cluster-known address connecting to avoid routing problems */ @@ -1366,7 +1369,7 @@ static int tcp_listen_for_all(void) sock = tcp_create_listen_sock(con, dlm_local_addr[0]); if (sock) { - add_sock(sock, con); + add_sock(sock, con, true); result = 0; } else { -- cgit v1.2.3 From d2fee58a3bb15b2b8f1eaff14aa3432cf0f35d8c Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 10 Oct 2016 09:19:52 -0400 Subject: dlm: remove lock_sock to avoid scheduling while atomic Before this patch, functions save_callbacks and restore_callbacks called function lock_sock and release_sock to prevent other processes from messing with the struct sock while the callbacks were saved and restored. However, function add_sock calls write_lock_bh prior to calling it save_callbacks, which disables preempts. So the call to lock_sock would try to schedule when we can't schedule. Signed-off-by: Bob Peterson Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'fs/dlm/lowcomms.c') diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 485494e5a28e..df680a26141b 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -519,24 +519,20 @@ out: /* Note: sk_callback_lock must be locked before calling this function. */ static void save_callbacks(struct connection *con, struct sock *sk) { - lock_sock(sk); con->orig_data_ready = sk->sk_data_ready; con->orig_state_change = sk->sk_state_change; con->orig_write_space = sk->sk_write_space; con->orig_error_report = sk->sk_error_report; - release_sock(sk); } static void restore_callbacks(struct connection *con, struct sock *sk) { write_lock_bh(&sk->sk_callback_lock); - lock_sock(sk); sk->sk_user_data = NULL; sk->sk_data_ready = con->orig_data_ready; sk->sk_state_change = con->orig_state_change; sk->sk_write_space = con->orig_write_space; sk->sk_error_report = con->orig_error_report; - release_sock(sk); write_unlock_bh(&sk->sk_callback_lock); } -- cgit v1.2.3 From 26c1ec2fe410ba861f15ebbfc9f44f907a41b6ff Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Sat, 22 Oct 2016 14:37:36 +0000 Subject: dlm: fix error return code in sctp_accept_from_sock() Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/dlm/lowcomms.c') diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index df680a26141b..7d398d300e97 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -879,7 +879,8 @@ static int sctp_accept_from_sock(struct connection *con) } make_sockaddr(&prim.ssp_addr, 0, &addr_len); - if (addr_to_nodeid(&prim.ssp_addr, &nodeid)) { + ret = addr_to_nodeid(&prim.ssp_addr, &nodeid); + if (ret) { unsigned char *b = (unsigned char *)&prim.ssp_addr; log_print("reject connect from unknown addr"); -- cgit v1.2.3