From 1a70855e25e8dd6242032df6f63f4b239f64f422 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Mon, 24 Jul 2017 11:19:20 -0500 Subject: [PATCH 1/9] Use static strings for epicsInterruptContextMessage() The callbackRequest() routine was passing a stack-allocated string to epicsInterruptContextMessage() but on RTEMS the pointer is queued without copying the string. This fix uses static strings for the 3 messages instead. Fixes LP: #1705219 --- src/db/callback.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/db/callback.c b/src/db/callback.c index 4e364b181..c387d79bb 100644 --- a/src/db/callback.c +++ b/src/db/callback.c @@ -60,6 +60,10 @@ static void *exitCallback; static char *threadName[NUM_CALLBACK_PRIORITIES] = { "cbLow", "cbMedium", "cbHigh" }; +#define FULL_MSG(name) "callbackRequest: " name " ring buffer full\n" +static char *fullMessage[NUM_CALLBACK_PRIORITIES] = { + FULL_MSG("cbLow"), FULL_MSG("cbMedium"), FULL_MSG("cbHigh") +}; static unsigned int threadPriority[NUM_CALLBACK_PRIORITIES] = { epicsThreadPriorityScanLow - 1, epicsThreadPriorityScanLow + 4, @@ -168,11 +172,7 @@ void callbackRequest(CALLBACK *pcallback) epicsInterruptUnlock(lockKey); if (!pushOK) { - char msg[48] = "callbackRequest: "; - - strcat(msg, threadName[priority]); - strcat(msg, " ring buffer full\n"); - epicsInterruptContextMessage(msg); + epicsInterruptContextMessage(fullMessage[priority]); ringOverflow[priority] = TRUE; } epicsEventSignal(callbackSem[priority]); From 0fc770166c0592232bb5d31ad90f37948d35228b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 12:18:01 +0200 Subject: [PATCH 2/9] rsrv: avoid possible overflow in vsend_err() Accounting of message size doesn't take into account space used by header of failed message (16 or 24 bytes). This would allow a theoretical really long error message to overflow the send buffer by 16 or 24 bytes. --- src/rsrv/camessage.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rsrv/camessage.c b/src/rsrv/camessage.c index df1ce65b2..160bc2138 100644 --- a/src/rsrv/camessage.c +++ b/src/rsrv/camessage.c @@ -216,10 +216,10 @@ va_list args /* * add their context string into the protocol */ - localStatus = epicsVsnprintf ( pMsgString, maxDiagLen, pformat, args ); + localStatus = epicsVsnprintf ( pMsgString, maxDiagLen - size, pformat, args ); if ( localStatus >= 1 ) { unsigned diagLen = ( unsigned ) localStatus; - if ( diagLen < maxDiagLen ) { + if ( diagLen < maxDiagLen - size ) { size += (ca_uint32_t) (diagLen + 1u); } else { @@ -227,7 +227,7 @@ va_list args "caserver: vsend_err: epicsVsnprintf detected " "error message truncation, pFormat = \"%s\"\n", pformat ); - size += maxDiagLen; + size = maxDiagLen; pMsgString [ maxDiagLen - 1 ] = '\0'; } } From 322f7a97dec445d35cd1f53e273a44cc4020b39f Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 12:28:23 +0200 Subject: [PATCH 3/9] rsrv: add some comments --- src/rsrv/caserverio.c | 3 +++ src/rsrv/server.h | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/rsrv/caserverio.c b/src/rsrv/caserverio.c index 387bf210b..b6cb4b92c 100644 --- a/src/rsrv/caserverio.c +++ b/src/rsrv/caserverio.c @@ -37,6 +37,9 @@ * cas_send_bs_msg() * * (channel access server send message) + * + * + * Set lock_needed=1 unless SEND_LOCK() is held by caller */ void cas_send_bs_msg ( struct client *pclient, int lock_needed ) { diff --git a/src/rsrv/server.h b/src/rsrv/server.h index e4b947ea9..7edf62661 100644 --- a/src/rsrv/server.h +++ b/src/rsrv/server.h @@ -62,8 +62,10 @@ typedef struct caHdrLargeArray { enum messageBufferType { mbtUDP, mbtSmallTCP, mbtLargeTCP }; struct message_buffer { char *buf; + /*! points to first filled byte in buffer */ unsigned stk; unsigned maxstk; + /*! points to first unused byte in buffer (after filled bytes) */ unsigned cnt; enum messageBufferType type; }; @@ -72,7 +74,9 @@ extern epicsThreadPrivateId rsrvCurrentClient; typedef struct client { ELLNODE node; + /*! guarded by SEND_LOCK() aka. client::lock */ struct message_buffer send; + /*! accessed by receive thread w/o locks cf. camsgtask() */ struct message_buffer recv; epicsMutexId lock; epicsMutexId putNotifyLock; From 1f8cb740f11f37faef8399b53cc17b61716dc651 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 12:29:03 +0200 Subject: [PATCH 4/9] rsrv: drop un-commited VERSION message This is a no-op as cas_commit_msg() isn't called. A VERSION message is already queued during create_tcp_client(). --- src/rsrv/camsgtask.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/rsrv/camsgtask.c b/src/rsrv/camsgtask.c index c1f112961..ca00cd62e 100644 --- a/src/rsrv/camsgtask.c +++ b/src/rsrv/camsgtask.c @@ -46,19 +46,6 @@ void camsgtask ( void *pParm ) casAttachThreadToClient ( client ); - /* - * send the server's minor version number to the client - */ - status = cas_copy_in_header ( client, CA_PROTO_VERSION, 0, - 0, CA_MINOR_PROTOCOL_REVISION, 0, 0, 0 ); - if ( status != ECA_NORMAL ) { - LOCK_CLIENTQ; - ellDelete ( &clientQ, &client->node ); - UNLOCK_CLIENTQ; - destroy_tcp_client ( client ); - return; - } - while (castcp_ctl == ctlRun && !client->disconnect) { /* From 619a99bf997984fa067a33551a86b1ea8282bd15 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 12:35:48 +0200 Subject: [PATCH 5/9] rsrv: missing send lock around send_err() --- src/rsrv/camessage.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rsrv/camessage.c b/src/rsrv/camessage.c index 160bc2138..44346ec77 100644 --- a/src/rsrv/camessage.c +++ b/src/rsrv/camessage.c @@ -2528,8 +2528,10 @@ int camessage ( struct client *client ) * aligned payloads */ if ( msgsize & 0x7 ) { + SEND_LOCK(client); send_err ( &msg, ECA_INTERNAL, client, "CAS: Missaligned protocol rejected" ); + SEND_UNLOCK(client); log_header ( "CAS: Missaligned protocol rejected", client, &msg, 0, nmsg ); status = RSRV_ERROR; @@ -2545,9 +2547,11 @@ int camessage ( struct client *client ) if ( msgsize > client->recv.maxstk ) { casExpandRecvBuffer ( client, msgsize ); if ( msgsize > client->recv.maxstk ) { + SEND_LOCK(client); send_err ( &msg, ECA_TOLARGE, client, "CAS: Server unable to load large request message. Max bytes=%lu", rsrvSizeofLargeBufTCP ); + SEND_UNLOCK(client); log_header ( "CAS: server unable to load large request message", client, &msg, 0, nmsg ); assert ( client->recv.cnt <= client->recv.maxstk ); From 4b272cc0cf9a827cb014de41926e6a3f4c98fa03 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 12:38:14 +0200 Subject: [PATCH 6/9] rsrv: locking in cas_send_bs_msg() Must lock around "pclient->send.stk = 0u;" --- src/rsrv/caserverio.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rsrv/caserverio.c b/src/rsrv/caserverio.c index b6cb4b92c..f84412213 100644 --- a/src/rsrv/caserverio.c +++ b/src/rsrv/caserverio.c @@ -45,6 +45,10 @@ void cas_send_bs_msg ( struct client *pclient, int lock_needed ) { int status; + if ( lock_needed ) { + SEND_LOCK ( pclient ); + } + if ( CASDEBUG > 2 && pclient->send.stk ) { errlogPrintf ( "CAS: Sending a message of %d bytes\n", pclient->send.stk ); } @@ -55,13 +59,11 @@ void cas_send_bs_msg ( struct client *pclient, int lock_needed ) pclient->sock, (unsigned) pclient->addr.sin_addr.s_addr ); } pclient->send.stk = 0u; + if(lock_needed) + SEND_UNLOCK(pclient); return; } - if ( lock_needed ) { - SEND_LOCK ( pclient ); - } - while ( pclient->send.stk && ! pclient->disconnect ) { status = send ( pclient->sock, pclient->send.buf, pclient->send.stk, 0 ); if ( status >= 0 ) { From 603331e7a5d9b9b8672b1bd16a1588fb6a64dcf8 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 12:58:25 +0200 Subject: [PATCH 7/9] rsrv: flush any queued messages before forced disconnect Avoid loss of various ERROR messages which camessage() has queued. --- src/rsrv/camsgtask.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rsrv/camsgtask.c b/src/rsrv/camsgtask.c index ca00cd62e..9cc3a294c 100644 --- a/src/rsrv/camsgtask.c +++ b/src/rsrv/camsgtask.c @@ -137,6 +137,9 @@ void camsgtask ( void *pParm ) } else { char buf[64]; + + /* flush any queued messages before shutdown */ + cas_send_bs_msg(client, 1); client->recv.cnt = 0ul; From 546df1c1f0d66795e9106c0ec20e429dcc895d15 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 31 Jul 2017 16:28:28 +0200 Subject: [PATCH 8/9] rsrv: export CASDEBUG to iocsh --- src/misc/base.dbd | 3 +++ src/rsrv/rsrvIocRegister.c | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/misc/base.dbd b/src/misc/base.dbd index ab10dae77..7cecf8c7a 100644 --- a/src/misc/base.dbd +++ b/src/misc/base.dbd @@ -43,6 +43,9 @@ include "devSoft.dbd" registrar(asSub) variable(asCaDebug,int) +# CA server debug flag (very verbose) range[0,5] +variable(CASDEBUG,int) + # dbStaticLib settings variable(dbRecordsOnceOnly,int) variable(dbBptNotMonotonic,int) diff --git a/src/rsrv/rsrvIocRegister.c b/src/rsrv/rsrvIocRegister.c index a713a0bd7..170780741 100644 --- a/src/rsrv/rsrvIocRegister.c +++ b/src/rsrv/rsrvIocRegister.c @@ -7,11 +7,15 @@ * in file LICENSE that is included with this distribution. \*************************************************************************/ +#include "osiSock.h" #include "iocsh.h" #define epicsExportSharedSymbols #include "rsrv.h" +#include "server.h" #include "rsrvIocRegister.h" +#include "epicsExport.h" + /* casr */ static const iocshArg casrArg0 = { "level",iocshArgInt}; static const iocshArg * const casrArgs[1] = {&casrArg0}; @@ -21,8 +25,9 @@ static void casrCallFunc(const iocshArgBuf *args) casr(args[0].ival); } - void epicsShareAPI rsrvIocRegister(void) { iocshRegister(&casrFuncDef,casrCallFunc); } + +epicsExportAddress(int, CASDEBUG); From 5c8e5c52ef80ee02e59be2bd9a324532d2b7fe0e Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 1 Aug 2017 10:32:09 +0200 Subject: [PATCH 9/9] rsrv: fix recv() error handling on WIN32 For WIN32 osiSockIoctl_t is unsigned, so > osiSockIoctl_t nchars = recv(... is casting signed -> unsigned which treats errors as success. --- src/rsrv/camsgtask.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/rsrv/camsgtask.c b/src/rsrv/camsgtask.c index 9cc3a294c..d1aae0e81 100644 --- a/src/rsrv/camsgtask.c +++ b/src/rsrv/camsgtask.c @@ -41,17 +41,18 @@ void camsgtask ( void *pParm ) { struct client *client = (struct client *) pParm; - int nchars; - int status; casAttachThreadToClient ( client ); while (castcp_ctl == ctlRun && !client->disconnect) { + osiSockIoctl_t check_nchars; + long nchars; + int status; /* * allow message to batch up if more are comming */ - status = socket_ioctl (client->sock, FIONREAD, &nchars); + status = socket_ioctl (client->sock, FIONREAD, &check_nchars); if (status < 0) { char sockErrBuf[64]; @@ -61,7 +62,7 @@ void camsgtask ( void *pParm ) sockErrBuf); cas_send_bs_msg(client, TRUE); } - else if (nchars == 0){ + else if (check_nchars == 0){ cas_send_bs_msg(client, TRUE); }