From 12812dca0f852178d8cc3813e4056b396a8f3ab5 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Wed, 7 Sep 2022 12:44:33 -0700 Subject: Handle arrays too large to fit in iceConn buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes numerous gcc warnings of the form: sm_client.c: In function ‘SmcOpenConnection’: SMlibint.h:109:25: warning: potential null pointer dereference [-Wnull-dereference] *((CARD32 *) _pBuf) = _val; \ SMlibint.h:160:5: note: in expansion of macro ‘STORE_CARD32’ STORE_CARD32 (_pBuf, (CARD32) _len); \ ^~~~~~~~~~~~ sm_client.c:207:5: note: in expansion of macro ‘STORE_ARRAY8’ STORE_ARRAY8 (pData, len, previousId); ^~~~~~~~~~~~ v2: Raise required libICE version to 1.1.0 to get the updated IceGetHeaderExtra macro definition needed for this to work correctly. Signed-off-by: Alan Coopersmith --- configure.ac | 2 +- src/SMlibint.h | 13 ++++++++++++ src/sm_client.c | 63 ++++++++++++++++++++++++++++++++++++++++++-------------- src/sm_manager.c | 10 ++++++--- 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/configure.ac b/configure.ac index 3635d6b..09c14a0 100644 --- a/configure.ac +++ b/configure.ac @@ -24,7 +24,7 @@ XORG_WITH_XSLTPROC XORG_CHECK_SGML_DOCTOOLS(1.8) # Obtain compiler/linker options for dependencies -PKG_CHECK_MODULES(SM, [ice >= 1.0.5] xproto xtrans) +PKG_CHECK_MODULES(SM, [ice >= 1.1.0] xproto xtrans) # Needed to check for TCP & IPv6 support and set flags appropriately XTRANS_CONNECTION_FLAGS diff --git a/src/SMlibint.h b/src/SMlibint.h index 94f13e9..197a561 100644 --- a/src/SMlibint.h +++ b/src/SMlibint.h @@ -182,6 +182,19 @@ in this Software without prior written authorization from The Open Group. } \ } +/* + * Send an ARRAY8 that doesn't fit in the iceConn send buffer. + */ +#define SEND_ARRAY8(_iceConn, _len, _array8) \ +{ \ + char _padding[7] = { 0 }; \ + CARD32 _array_len = (CARD32) _len; \ + IceWriteData32 (_iceConn, 4, &_array_len); \ + if (_len) \ + IceSendData (_iceConn, _len, (char *) _array8); \ + IceSendData (_iceConn, PAD64 (4 + _len), _padding); \ +} + /* * Client replies not processed by callbacks (we block for them). diff --git a/src/sm_client.c b/src/sm_client.c index 3c939b8..4552819 100644 --- a/src/sm_client.c +++ b/src/sm_client.c @@ -204,9 +204,13 @@ SmcOpenConnection(char *networkIdsList, SmPointer context, SIZEOF (smRegisterClientMsg), WORD64COUNT (extra), smRegisterClientMsg, pMsg, pData); - STORE_ARRAY8 (pData, len, previousId); - - IceFlush (iceConn); + if (pData != NULL) { + STORE_ARRAY8 (pData, len, previousId); + IceFlush (iceConn); + } + else { + SEND_ARRAY8 (iceConn, len, previousId); + } replyWait.sequence_of_request = IceLastSentSequenceNumber (iceConn); replyWait.major_opcode_of_request = _SmcOpcode; @@ -260,9 +264,13 @@ SmcOpenConnection(char *networkIdsList, SmPointer context, SIZEOF (smRegisterClientMsg), WORD64COUNT (extra), smRegisterClientMsg, pMsg, pData); - STORE_ARRAY8 (pData, 0, ""); - - IceFlush (iceConn); + if (pData != NULL) { + STORE_ARRAY8 (pData, 0, ""); + IceFlush (iceConn); + } + else { + SEND_ARRAY8 (iceConn, 0, ""); + } replyWait.sequence_of_request = IceLastSentSequenceNumber (iceConn); @@ -296,13 +304,24 @@ SmcCloseConnection(SmcConn smcConn, int count, char **reasonMsgs) SIZEOF (smCloseConnectionMsg), WORD64COUNT (extra), smCloseConnectionMsg, pMsg, pData); - STORE_CARD32 (pData, (CARD32) count); - pData += 4; + if (pData != NULL) { + STORE_CARD32 (pData, (CARD32) count); + STORE_CARD32 (pData, (CARD32) 0); /* padding */ - for (i = 0; i < count; i++) - STORE_ARRAY8 (pData, strlen (reasonMsgs[i]), reasonMsgs[i]); + for (i = 0; i < count; i++) + STORE_ARRAY8 (pData, strlen (reasonMsgs[i]), reasonMsgs[i]); - IceFlush (iceConn); + IceFlush (iceConn); + } else { + CARD32 count_header[2] = { + (CARD32) count, + (CARD32) 0 /* padding */ + }; + IceWriteData32 (iceConn, 8, count_header); + + for (i = 0; i < count; i++) + SEND_ARRAY8 (iceConn, strlen (reasonMsgs[i]), reasonMsgs[i]); + } IceProtocolShutdown (iceConn, _SmcOpcode); IceSetShutdownNegotiation (iceConn, False); @@ -412,13 +431,25 @@ SmcDeleteProperties(SmcConn smcConn, int numProps, char **propNames) SIZEOF (smDeletePropertiesMsg), WORD64COUNT (extra), smDeletePropertiesMsg, pMsg, pData); - STORE_CARD32 (pData, numProps); - pData += 4; + if (pData != NULL) { + STORE_CARD32 (pData, (CARD32) numProps); + STORE_CARD32 (pData, (CARD32) 0); /* padding */ - for (i = 0; i < numProps; i++) - STORE_ARRAY8 (pData, strlen (propNames[i]), propNames[i]); + for (i = 0; i < numProps; i++) + STORE_ARRAY8 (pData, strlen (propNames[i]), propNames[i]); - IceFlush (iceConn); + IceFlush (iceConn); + } + else { + CARD32 count_header[2] = { + (CARD32) numProps, + (CARD32) 0 /* padding */ + }; + IceWriteData32 (iceConn, 8, count_header); + + for (i = 0; i < numProps; i++) + SEND_ARRAY8 (iceConn, strlen (propNames[i]), propNames[i]); + } } diff --git a/src/sm_manager.c b/src/sm_manager.c index 7d0e583..81e7d2f 100644 --- a/src/sm_manager.c +++ b/src/sm_manager.c @@ -198,9 +198,13 @@ SmsRegisterClientReply(SmsConn smsConn, char *clientId) SIZEOF (smRegisterClientReplyMsg), WORD64COUNT (extra), smRegisterClientReplyMsg, pMsg, pData); - STORE_ARRAY8 (pData, strlen (clientId), clientId); - - IceFlush (iceConn); + if (pData != NULL) { + STORE_ARRAY8 (pData, strlen (clientId), clientId); + IceFlush (iceConn); + } + else { + SEND_ARRAY8 (iceConn, strlen (clientId), clientId); + } return (1); } -- cgit v1.2.1