Date: Sun, 17 May 2020 22:05:26 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r361144 - in projects/nfs-over-tls/sys/rpc: . rpcsec_tls Message-ID: <202005172205.04HM5Q4X042433@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rmacklem Date: Sun May 17 22:05:25 2020 New Revision: 361144 URL: https://svnweb.freebsd.org/changeset/base/361144 Log: Assorted error handling fixups plus a fix for client side handling of non-application data. The error handling changes more clearly separate upcall errors from RPC errors. The cases where the disconnect upcall fails now assumes that the socket has been closed by a crashed/restarted daemon and does not attempt to close the socket again. The ct_rcvstate variable becomes a set of flag bits, so a flag bit can be set to tell clnt_vc_dotlsupcall() to call clnt_vc_soupcall() when a socket upcall needed to just return due to a upcall to the rpctlscd daemon was in progress. Modified: projects/nfs-over-tls/sys/rpc/clnt_vc.c projects/nfs-over-tls/sys/rpc/krpc.h projects/nfs-over-tls/sys/rpc/rpcsec_tls/rpctls_impl.c projects/nfs-over-tls/sys/rpc/svc_vc.c Modified: projects/nfs-over-tls/sys/rpc/clnt_vc.c ============================================================================== --- projects/nfs-over-tls/sys/rpc/clnt_vc.c Sun May 17 21:54:59 2020 (r361143) +++ projects/nfs-over-tls/sys/rpc/clnt_vc.c Sun May 17 22:05:25 2020 (r361144) @@ -159,7 +159,7 @@ clnt_vc_create( ct->ct_closing = FALSE; ct->ct_closed = FALSE; ct->ct_upcallrefs = 0; - ct->ct_rcvstate = RCVNORMAL; + ct->ct_rcvstate = RPCRCVSTATE_NORMAL; if ((so->so_state & (SS_ISCONNECTED|SS_ISCONFIRMING)) == 0) { error = soconnect(so, raddr, curthread); @@ -278,6 +278,7 @@ clnt_vc_create( ct->ct_raw = NULL; ct->ct_record = NULL; ct->ct_record_resid = 0; + ct->ct_sslrefno = 0; TAILQ_INIT(&ct->ct_pending); return (cl); @@ -424,8 +425,8 @@ call_again: * Wait until any upcall is completed. */ clnt_vc_dotlsupcall(ct); - while (ct->ct_rcvstate != RCVNORMAL && - ct->ct_rcvstate != RCVNONAPPDATA) + while ((ct->ct_rcvstate & (RPCRCVSTATE_NORMAL | + RPCRCVSTATE_NONAPPDATA)) == 0) msleep(&ct->ct_rcvstate, &ct->ct_lock, 0, "rpcrcvst", hz); TAILQ_INSERT_TAIL(&ct->ct_pending, cr, cr_link); @@ -802,10 +803,13 @@ printf("backch tls=0x%x xprt=%p\n", xprt->xp_tls, xprt break; case CLSET_BLOCKRCV: - if (*(int *) info) - ct->ct_rcvstate = TLSHANDSHAKE; - else - ct->ct_rcvstate = RCVNORMAL; + if (*(int *) info) { + ct->ct_rcvstate &= ~RPCRCVSTATE_NORMAL; + ct->ct_rcvstate |= RPCRCVSTATE_TLSHANDSHAKE; + } else { + ct->ct_rcvstate &= ~RPCRCVSTATE_TLSHANDSHAKE; + ct->ct_rcvstate |= RPCRCVSTATE_NORMAL; + } break; default: @@ -900,13 +904,17 @@ clnt_vc_destroy(CLIENT *cl) mtx_destroy(&ct->ct_lock); if (so) { - stat = RPC_FAILED; - reterr = RPCTLSERR_OK; - if (ct->ct_sslrefno != 0) + if (ct->ct_sslrefno != 0) { + /* + * If the upcall fails, the socket has + * probably been closed via the rpctlscd + * daemon having crashed or been + * restarted. + */ stat = rpctls_cl_disconnect(ct->ct_sslsec, ct->ct_sslusec, ct->ct_sslrefno, &reterr); - if (stat != RPC_SUCCESS || reterr == RPCTLSERR_NOCLOSE) { + } else { soshutdown(so, SHUT_WR); soclose(so); } @@ -955,8 +963,12 @@ clnt_vc_soupcall(struct socket *so, void *arg, int wai * the socket via openssl library calls. */ mtx_lock(&ct->ct_lock); - if (ct->ct_rcvstate != RCVNORMAL && ct->ct_rcvstate != - RCVNONAPPDATA) { + if ((ct->ct_rcvstate & (RPCRCVSTATE_NORMAL | + RPCRCVSTATE_NONAPPDATA)) == 0) { + /* Mark that a socket upcall needs to be done. */ + if ((ct->ct_rcvstate & (RPCRCVSTATE_UPCALLNEEDED | + RPCRCVSTATE_UPCALLINPROG)) != 0) + ct->ct_rcvstate |= RPCRCVSTATE_SOUPCALLNEEDED; mtx_unlock(&ct->ct_lock); return (SU_OK); } @@ -982,7 +994,8 @@ clnt_vc_soupcall(struct socket *so, void *arg, int wai uio.uio_td = curthread; m2 = m = NULL; rcvflag = MSG_DONTWAIT | MSG_SOCALLBCK; - if (ct->ct_sslrefno != 0 && ct->ct_rcvstate == RCVNORMAL) { + if (ct->ct_sslrefno != 0 && (ct->ct_rcvstate & + RPCRCVSTATE_NORMAL) != 0) { rcvflag |= MSG_TLSAPPDATA; ctrlp = NULL; } else @@ -1021,7 +1034,7 @@ clnt_vc_soupcall(struct socket *so, void *arg, int wai if (ct->ct_sslrefno != 0 && error == ENXIO) { /* Disable reception, marking an upcall needed. */ mtx_lock(&ct->ct_lock); - ct->ct_rcvstate = UPCALLNEEDED; + ct->ct_rcvstate |= RPCRCVSTATE_UPCALLNEEDED; /* * If an upcall in needed, wake up all thread(s) * in clnt_vc_call() so that one of them can do it. @@ -1057,16 +1070,18 @@ if (m2->m_next != NULL) printf("EEK! list of controls\ memcpy(&tgr, CMSG_DATA(cmsg), sizeof(tgr)); /* * This should have been handled by - * setting ct_rcvstate == UPCALLNEEDED, - * but if not, all we can do is toss - * it away. + * setting RPCRCVSTATE_UPCALLNEEDED in + * ct_rcvstate but if not, all we can do + * is toss it away. */ if (tgr.tls_type != TLS_RLTYPE_APP) { printf("Got weird type=%d\n", tgr.tls_type); m_freem(m); m_free(m2); mtx_lock(&ct->ct_lock); - ct->ct_rcvstate = RCVNORMAL; + ct->ct_rcvstate &= + ~RPCRCVSTATE_NONAPPDATA; + ct->ct_rcvstate |= RPCRCVSTATE_NORMAL; mtx_unlock(&ct->ct_lock); continue; } @@ -1289,16 +1304,29 @@ clnt_vc_dotlsupcall(struct ct_data *ct) uint32_t reterr; mtx_assert(&ct->ct_lock, MA_OWNED); - if (ct->ct_rcvstate == UPCALLNEEDED) { - ct->ct_rcvstate = UPCALLINPROG; - mtx_unlock(&ct->ct_lock); - ret = rpctls_cl_handlerecord(ct->ct_sslsec, ct->ct_sslusec, - ct->ct_sslrefno, &reterr); - mtx_lock(&ct->ct_lock); - if (ret == RPC_SUCCESS && reterr == RPCTLSERR_OK) - ct->ct_rcvstate = RCVNORMAL; - else - ct->ct_rcvstate = RCVNONAPPDATA; - wakeup(&ct->ct_rcvstate); + while ((ct->ct_rcvstate & (RPCRCVSTATE_UPCALLNEEDED | + RPCRCVSTATE_SOUPCALLNEEDED)) != 0) { + if ((ct->ct_rcvstate & RPCRCVSTATE_UPCALLNEEDED) != 0) { + ct->ct_rcvstate &= ~RPCRCVSTATE_UPCALLNEEDED; + ct->ct_rcvstate |= RPCRCVSTATE_UPCALLINPROG; + mtx_unlock(&ct->ct_lock); + ret = rpctls_cl_handlerecord(ct->ct_sslsec, ct->ct_sslusec, + ct->ct_sslrefno, &reterr); + mtx_lock(&ct->ct_lock); + ct->ct_rcvstate &= ~RPCRCVSTATE_UPCALLINPROG; + if (ret == RPC_SUCCESS && reterr == RPCTLSERR_OK) + ct->ct_rcvstate |= RPCRCVSTATE_NORMAL; + else + ct->ct_rcvstate |= RPCRCVSTATE_NONAPPDATA; + wakeup(&ct->ct_rcvstate); + } + if ((ct->ct_rcvstate & RPCRCVSTATE_SOUPCALLNEEDED) != 0) { + ct->ct_rcvstate &= ~RPCRCVSTATE_SOUPCALLNEEDED; + mtx_unlock(&ct->ct_lock); + SOCKBUF_LOCK(&ct->ct_socket->so_rcv); + clnt_vc_soupcall(ct->ct_socket, ct, M_NOWAIT); + SOCKBUF_UNLOCK(&ct->ct_socket->so_rcv); + mtx_lock(&ct->ct_lock); + } } } Modified: projects/nfs-over-tls/sys/rpc/krpc.h ============================================================================== --- projects/nfs-over-tls/sys/rpc/krpc.h Sun May 17 21:54:59 2020 (r361143) +++ projects/nfs-over-tls/sys/rpc/krpc.h Sun May 17 22:05:25 2020 (r361144) @@ -82,13 +82,13 @@ struct rc_data { bool rc_tls; /* Enable TLS on connection */ }; -enum clnt_rcvstate { - RCVNORMAL = 0, /* Normal reception. */ - RCVNONAPPDATA = 1, /* Reception of a non-application record. */ - TLSHANDSHAKE = 2, /* Reception blocked for TLS handshake. */ - UPCALLNEEDED = 3, /* Upcall to rpctlscd needed. */ - UPCALLINPROG = 4 /* Upcall to rpctlscd in progress. */ -}; +/* Bits for ct_rcvstate. */ +#define RPCRCVSTATE_NORMAL 0x01 /* Normal reception. */ +#define RPCRCVSTATE_NONAPPDATA 0x02 /* Reception of a non-application record. */ +#define RPCRCVSTATE_TLSHANDSHAKE 0x04 /* Reception blocked for TLS handshake. */ +#define RPCRCVSTATE_UPCALLNEEDED 0x08 /* Upcall to rpctlscd needed. */ +#define RPCRCVSTATE_UPCALLINPROG 0x10 /* Upcall to rpctlscd in progress. */ +#define RPCRCVSTATE_SOUPCALLNEEDED 0x20 /* Socket upcall needed. */ struct ct_data { struct mtx ct_lock; @@ -114,7 +114,7 @@ struct ct_data { uint64_t ct_sslsec; /* RPC-over-TLS connection. */ uint64_t ct_sslusec; uint64_t ct_sslrefno; - enum clnt_rcvstate ct_rcvstate; /* Block receiving for TLS upcalls */ + uint32_t ct_rcvstate; /* Handle receiving for TLS upcalls */ struct mbuf *ct_raw; /* Raw mbufs recv'd */ }; Modified: projects/nfs-over-tls/sys/rpc/rpcsec_tls/rpctls_impl.c ============================================================================== --- projects/nfs-over-tls/sys/rpc/rpcsec_tls/rpctls_impl.c Sun May 17 21:54:59 2020 (r361143) +++ projects/nfs-over-tls/sys/rpc/rpcsec_tls/rpctls_impl.c Sun May 17 22:05:25 2020 (r361144) @@ -404,8 +404,10 @@ rpctls_cl_handlerecord(uint64_t sec, uint64_t usec, ui printf("In rpctls_cl_handlerecord\n"); cl = rpctls_connect_client(); printf("handlerecord_client=%p\n", cl); - if (cl == NULL) - return (RPC_AUTHERROR); + if (cl == NULL) { + *reterr = RPCTLSERR_NOSSL; + return (RPC_SUCCESS); + } /* Do the handlerecord upcall. */ arg.sec = sec; @@ -431,8 +433,10 @@ rpctls_srv_handlerecord(uint64_t sec, uint64_t usec, u printf("In rpctls_srv_handlerecord\n"); cl = rpctls_server_client(); printf("srv handlerecord_client=%p\n", cl); - if (cl == NULL) - return (RPC_AUTHERROR); + if (cl == NULL) { + *reterr = RPCTLSERR_NOSSL; + return (RPC_SUCCESS); + } /* Do the handlerecord upcall. */ arg.sec = sec; @@ -459,8 +463,10 @@ rpctls_cl_disconnect(uint64_t sec, uint64_t usec, uint printf("In rpctls_cl_disconnect\n"); cl = rpctls_connect_client(); printf("disconnect_client=%p\n", cl); - if (cl == NULL) - return (RPC_AUTHERROR); + if (cl == NULL) { + *reterr = RPCTLSERR_NOSSL; + return (RPC_SUCCESS); + } /* Do the disconnect upcall. */ arg.sec = sec; @@ -486,8 +492,10 @@ rpctls_srv_disconnect(uint64_t sec, uint64_t usec, uin printf("In rpctls_srv_disconnect\n"); cl = rpctls_server_client(); printf("srv disconnect_client=%p\n", cl); - if (cl == NULL) - return (RPC_AUTHERROR); + if (cl == NULL) { + *reterr = RPCTLSERR_NOSSL; + return (RPC_SUCCESS); + } /* Do the disconnect upcall. */ arg.sec = sec; Modified: projects/nfs-over-tls/sys/rpc/svc_vc.c ============================================================================== --- projects/nfs-over-tls/sys/rpc/svc_vc.c Sun May 17 21:54:59 2020 (r361143) +++ projects/nfs-over-tls/sys/rpc/svc_vc.c Sun May 17 22:05:25 2020 (r361144) @@ -455,12 +455,17 @@ svc_vc_destroy_common(SVCXPRT *xprt) uint32_t reterr; if (xprt->xp_socket) { - stat = RPC_FAILED; - if ((xprt->xp_tls & RPCTLS_FLAGS_HANDSHAKE) != 0) + if ((xprt->xp_tls & RPCTLS_FLAGS_HANDSHAKE) != 0) { + /* + * If the upcall fails, the socket has + * probably been closed via the rpctlssd + * daemon having crashed or been + * restarted. + */ stat = rpctls_srv_disconnect(xprt->xp_sslsec, xprt->xp_sslusec, xprt->xp_sslrefno, &reterr); - if (stat != RPC_SUCCESS || reterr == RPCTLSERR_NOCLOSE) + } else (void)soclose(xprt->xp_socket); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005172205.04HM5Q4X042433>