Modifications to resolve issues found during self-code review.

This commit is contained in:
Juan Carlos Luciani
2006-12-08 05:45:03 +00:00
parent 9a0426279c
commit 8ade751650
34 changed files with 524 additions and 268 deletions

View File

@@ -88,8 +88,9 @@ linux/client/test/testClient.c.
SECURITY CONSIDERATIONS
IpcLibs does not provide any security features beyond what is provided by the protocol stacks for
tcp/ip and Domain sockets communications.
tcp/ip and Domain sockets communications. IpcLibs does not directly perform any uid/gid checks
when Domain socket communications are performed.
By leveraging the File System Access Control features, you can scope communications that occur over
Domain sockets to specific or groups of users.
Domain sockets to specific or groups of users.

View File

@@ -390,9 +390,9 @@ CChannel::connectionThread(
{
CChannel *pCChannel = *pSmartCChannel;
bool doneReceivingData = false;
unsigned long bytesReceived;
int32_t bytesReceived;
uint32_t reqId;
int payloadLength;
int32_t payloadLength;
unsigned long totalPayloadBytesReceived = 0;
char reqDataPktHdr[ReqDataPktHdrTemplate.length()];
char *pRecvBuff;
@@ -441,7 +441,7 @@ CChannel::connectionThread(
&payloadLength))
{
// Procced based on the packet type
switch (ChannelProto::getPktType(*reqDataPktHdr))
switch (ChannelProto::getPktType(*reqDataPktHdr, sizeof(reqDataPktHdr)))
{
case ChannelProto::ReqDataCarrierPacketType:
@@ -747,7 +747,7 @@ CChannel::submitReq(
uint32_t reqId,
ClientReq &clientReq,
char *pClientData,
int clientDataLen)
int32_t clientDataLen)
//
// Arguments:
//

View File

@@ -222,7 +222,7 @@ public:
int submitReq(uint32_t reqId,
ClientReq &clientReq,
char *pClientData,
int clientDataLen);
int32_t clientDataLen);
//
// Remove Request routine

View File

@@ -354,8 +354,6 @@ IpcClientCloseRemoteEndPoint(
DbgTrace(0, "IpcClientCloseRemoteEndPoint- Not initialized\n", 0);
}
exit:
DbgTrace(1, "IpcClientCloseRemoteEndPoint- End, status = %0X\n", retStatus);
return retStatus;
@@ -368,9 +366,9 @@ int
IpcClientSubmitReq(
IN uint32_t endPointHandle,
IN char *pClientData,
IN int clientDataLen,
IN int32_t clientDataLen,
INOUT char **ppServerData,
INOUT int *pServerDataLen)
INOUT int32_t *pServerDataLen)
//
// Arguments In: endPointHandle - Handle of the remote endpoint that will
// be the target of the request.
@@ -410,6 +408,15 @@ IpcClientSubmitReq(
DbgTrace(1, "IpcClientSubmitReq- Start\n", 0);
// Verify input parameters
if (pClientData == NULL
|| ppServerData == NULL
|| pServerDataLen == NULL)
{
DbgTrace(0, "IpcClientSubmitReq- Invalid parameter\n", 0);
goto exit;
}
// Verify that we have been initialized
if (svcInitialized)
{
@@ -425,7 +432,7 @@ IpcClientSubmitReq(
// the request.
SmartRemoteEndPoint *pSmartRemoteEndPoint = new SmartRemoteEndPoint(*(iter->second));
// Release our mutex before deleting the endpoint
// Release our mutex before using the remote endpoint
pthread_mutex_unlock(&clientMutex);
// Submit the request
@@ -450,6 +457,8 @@ IpcClientSubmitReq(
DbgTrace(0, "IpcClientSubmitReq- Not initialized\n", 0);
}
exit:
DbgTrace(1, "IpcClientSubmitReq- End, retStatus = %0X\n", retStatus);
return retStatus;
@@ -498,7 +507,7 @@ IpcClientInit(
DbgTrace(1, "IpcClientInit- Start\n", 0);
// Check input parameters
if (pAppName == NULL)
if (pName == NULL)
{
DbgTrace(0, "IpcClientInit- Invalid parameter\n", 0);
goto exit;

View File

@@ -142,7 +142,7 @@ ClientReq::~ClientReq(void)
void
ClientReq::processServerData(
char *pServerData,
int serverDataLength)
int32_t serverDataLength)
//
// Arguments:
//
@@ -230,7 +230,7 @@ ClientReq::processError(void)
int
ClientReq::waitForCompletion(
char **ppResponseData,
int *pResponseDataLength)
int32_t *pResponseDataLength)
//
// Arguments:
//

View File

@@ -52,7 +52,7 @@ class ClientReq
// Server Data
char *m_pServerData;
int m_serverDataLen;
int32_t m_serverDataLen;
// Flag indicating the state of the submitting
// thread.
@@ -114,7 +114,7 @@ public:
// Returns: Nothing.
//
void processServerData(char *pServerData,
int serverDataLength);
int32_t serverDataLength);
//
// Process Error routine
@@ -147,7 +147,7 @@ public:
// -1 == Request did not complete gracefully
//
int waitForCompletion(char **ppResponseData,
int *pResponseDataLength);
int32_t *pResponseDataLength);
//
// Completion status

View File

@@ -80,31 +80,41 @@ RemoteEndPoint::RemoteEndPoint(
{
DbgTrace(1, "RemoteEndPoint::RemoteEndPoint- Start, Obj = %0X\n", this);
// Initialize our mutex
pthread_mutex_init(&m_mutex, NULL);
// Verify that the specified path is not too long
if (strlen(pSocketFileName) < sizeof(sizeof(m_serverUnAddr.sun_path)))
{
// Initialize our mutex
pthread_mutex_init(&m_mutex, NULL);
// Set the necessary information in the m_serverUnAddr variable
m_serverUnAddr.sun_family = AF_UNIX;
strcpy(m_serverUnAddr.sun_path, pSocketFileName);
// Set the necessary information in the m_serverUnAddr variable
m_serverUnAddr.sun_family = AF_UNIX;
strncpy(m_serverUnAddr.sun_path, pSocketFileName, sizeof(m_serverUnAddr.sun_path) - 1);
// Set the necessary flags to indicate that DOMAIN sockets
// should be used for communications.
m_Use_PF_UNIX = true;
m_Use_AF_INET = false;
// Set the necessary flags to indicate that DOMAIN sockets
// should be used for communications.
m_Use_PF_UNIX = true;
m_Use_AF_INET = false;
// Setup the number of channels that we may have based on
// whether the application is multi-threaded or not.
if (multithreaded)
m_numCChannels = MAX_CHANNELS_PER_ENDPOINT;
// Setup the number of channels that we may have based on
// whether the application is multi-threaded or not.
if (multithreaded)
m_numCChannels = MAX_CHANNELS_PER_ENDPOINT;
else
m_numCChannels = 1;
// Instantiate entries in SmartCChannel vector
try {
for (int i = 0; i < m_numCChannels; i++)
m_cchannelVector.push_back(SmartCChannelPointer());
} catch (...) {
DbgTrace(0, "RemoteEndPoint::RemoteEndPoint- Exception caught while initializing the cchannelVector\n", 0);
pthread_mutex_destroy(&m_mutex);
throw bad_alloc();
}
}
else
m_numCChannels = 1;
// Instantiate entries in SmartCChannel vector
try {
for (int i = 0; i < m_numCChannels; i++)
m_cchannelVector.push_back(SmartCChannelPointer());
} catch (...) {
DbgTrace(0, "RemoteEndPoint::RemoteEndPoint- Exception caught while initializing the cchannelVector\n", 0);
{
DbgTrace(0, "RemoteEndPoint::RemoteEndPoint- Socket file path name too long\n", 0);
throw bad_alloc();
}
@@ -166,6 +176,7 @@ RemoteEndPoint::RemoteEndPoint(
m_cchannelVector.push_back(SmartCChannelPointer());
} catch (...) {
DbgTrace(0, "RemoteEndPoint::RemoteEndPoint- Exception caught while initializing the cchannelVector\n", 0);
pthread_mutex_destroy(&m_mutex);
throw bad_alloc();
}
@@ -259,7 +270,7 @@ RemoteEndPoint::getCChannel(void)
m_cchannelVector[channelSelector].setPointer(NULL);
}
CChannel *pCChannel;
CChannel *pCChannel = NULL;
try {
// Use the appropriate server address when instantiating
@@ -325,9 +336,9 @@ RemoteEndPoint::getCChannel(void)
int
RemoteEndPoint::submitReq(
char *pClientData,
int clientDataLen,
int32_t clientDataLen,
char **ppServerData,
int *pServerDataLen)
int32_t *pServerDataLen)
//
// Arguments:
//

View File

@@ -186,9 +186,9 @@ public:
// Note: The routine blocks until the request completes.
//
int submitReq(char *pClientData,
int clientDataLen,
int32_t clientDataLen,
char **ppServerData,
int *pServerDataLen);
int32_t *pServerDataLen);
};
typedef SmartPtr<RemoteEndPoint> SmartRemoteEndPoint;

View File

@@ -68,7 +68,7 @@ ChannelProto::buildReqDataPktHdr(
//
// Abstract:
//
// Notes:
// Notes: pPktHdr must point to a buffer of size ReqDataPktHdrTemple.length().
//
// L2
//=======================================================================--
@@ -81,16 +81,16 @@ ChannelProto::buildReqDataPktHdr(
// - Req Data Packet Header Format -
//
// ReqDataCarrierType
// ReqIdHdr value (value format=%0X)
// PayloadLengthHdr value (value format=%0X)
// ReqIdHdr value (value format=%08X)
// PayloadLengthHdr value (value format=%08X)
//
// Setup the necessary value strings
char wrkBuffer[10];
sprintf(wrkBuffer, "%0X", reqId);
sprintf(wrkBuffer, "%08X", reqId);
string reqIdValue = wrkBuffer;
sprintf(wrkBuffer, "%0X", payloadLength);
sprintf(wrkBuffer, "%08X", payloadLength);
string payloadLengthValue = wrkBuffer;
// Format the header.
@@ -141,7 +141,7 @@ ChannelProto::buildReqErrorPktHdr(
//
// Abstract:
//
// Notes:
// Notes: pPktHdr must point to a buffer of size ReqErrorPktHdrTemple.length().
//
// L2
//=======================================================================--
@@ -154,16 +154,16 @@ ChannelProto::buildReqErrorPktHdr(
// - Req Error Packet Header Format -
//
// ReqErrorCarrierType
// ReqIdHdr value (value format=%0X)
// PayloadLengthHdr value (value format=%0X)
// ReqIdHdr value (value format=%08X)
// PayloadLengthHdr value (value format=%08X)
//
// Setup the necessary value strings
char wrkBuffer[10];
sprintf(wrkBuffer, "%0X", reqId);
sprintf(wrkBuffer, "%08X", reqId);
string reqIdValue = wrkBuffer;
sprintf(wrkBuffer, "%0X", payloadLength);
sprintf(wrkBuffer, "%08X", payloadLength);
string payloadLengthValue = wrkBuffer;
// Format the header.
@@ -204,7 +204,8 @@ ChannelProto::buildReqErrorPktHdr(
//++=======================================================================
ChannelProto::PacketTypes
ChannelProto::getPktType(
char &buff)
char &buff,
int hdrLength)
//
// Arguments:
//
@@ -223,29 +224,47 @@ ChannelProto::getPktType(
// Find the end of the Channel Packet Type
char *pCurr = &buff;
while (*pCurr != '\r')
int bytesLeft = hdrLength;
bool endFound = false;
while (bytesLeft)
{
if (*pCurr == '\r')
{
endFound = true;
break;
}
pCurr ++;
// Found the end of the Channel Packet Type, now
// calculate its length.
int channelPktTypeLength = pCurr - &buff;
// Now start comparing
if (channelPktTypeLength == ReqDataCarrierType.length()
&& !memcmp(&buff, ReqDataCarrierType.c_str(), channelPktTypeLength))
{
// The type is Channel Req Data Carrier
packetType = ReqDataCarrierPacketType;
bytesLeft --;
}
else if (channelPktTypeLength == ReqErrorCarrierType.length()
&& !memcmp(&buff, ReqErrorCarrierType.c_str(), channelPktTypeLength))
if (endFound)
{
// The type is Channel Req Error Carrier
packetType = ReqErrorCarrierPacketType;
// Found the end of the Channel Packet Type, now
// calculate its length.
int channelPktTypeLength = pCurr - &buff;
// Now start comparing
if (channelPktTypeLength == ReqDataCarrierType.length()
&& !memcmp(&buff, ReqDataCarrierType.c_str(), channelPktTypeLength))
{
// The type is Channel Req Data Carrier
packetType = ReqDataCarrierPacketType;
}
else if (channelPktTypeLength == ReqErrorCarrierType.length()
&& !memcmp(&buff, ReqErrorCarrierType.c_str(), channelPktTypeLength))
{
// The type is Channel Req Error Carrier
packetType = ReqErrorCarrierPacketType;
}
else
{
DbgTrace(0, "ChannelProto::getPktType- No match found\n", 0);
}
}
else
{
DbgTrace(0, "ChannelProto::getPktType- No match found\n", 0);
DbgTrace(0, "ChannelProto::getPktType- Invalid header\n", 0);
}
DbgTrace(1, "ChannelProto::getPktType- End, type = %d\n", packetType);
@@ -283,7 +302,8 @@ ChannelProto::getReqIdAndPayloadLength(
char *pChannelHdr = NULL;
int bytesLeft = hdrLength;
// Skip the Channel Packet Type
// Skip the Channel Packet Type which should always
// be the first header.
while (bytesLeft >= 2)
{
if (*pCurr == '\r'
@@ -334,7 +354,17 @@ ChannelProto::getReqIdAndPayloadLength(
*(pCurr-2) = '\0';
// Convert the value to hex
*pReqId = strtoul(pValue, NULL, 16);
errno = 0;
unsigned long int value = strtoul(pValue, NULL, 16);
if (errno != 0
|| value > UINT32_MAX)
{
DbgTrace(0, "ChannelProto::getReqIdAndPayloadLength- Invalid reqId value, %s\n", pValue);
break;
}
// Use the value
*pReqId = (uint32_t) value;
// Undo the damage that we did
*(pCurr-2) = '\r';
@@ -353,7 +383,17 @@ ChannelProto::getReqIdAndPayloadLength(
*(pCurr-2) = '\0';
// Convert the value to hex
*pPayloadLength = strtoul(pValue, NULL, 16);
errno = 0;
long int value = strtol(pValue, NULL, 16);
if (errno != 0
|| value > INT32_MAX)
{
DbgTrace(0, "ChannelProto::getReqIdAndPayloadLength- Invalid payloadLength value, %s\n", pValue);
break;
}
// Use the value
*pPayloadLength = (int32_t) value;
// Undo the damage that we did
*(pCurr-2) = '\r';

View File

@@ -113,12 +113,16 @@ public:
// Parameters:
// buff (input) -
// Reference to buffer containing the packet data.
//
// hdrLength (input) -
// Length of the channel header.
//
// Abstract: Returns the type of the specified channel packet.
//
// Returns: Channel packet type.
//
static PacketTypes getPktType(char &buff);
static PacketTypes getPktType(char &buff,
int hdrLength);
//
// Get Req Id and Payload Length Values routine

View File

@@ -234,15 +234,15 @@ SChannel::connectionThread(
{
SChannel *pSChannel = *pSmartSChannel;
bool doneReceivingData = false;
unsigned long bytesReceived;
int32_t bytesReceived;
unsigned long bytesSent;
uint32_t reqId;
int payloadLength;
int32_t payloadLength;
unsigned long totalPayloadBytesReceived = 0;
char reqDataPktHdr[ReqDataPktHdrTemplate.length()];
char reqErrorPktHdr[ReqErrorPktHdrTemplate.length()];
char *pRecvBuff;
ServerReq *pServerReq;
ServerReq *pServerReq = NULL;
DbgTrace(1, "SChannel::connectionThread- Start, Obj = %0X\n", pSChannel);
@@ -285,7 +285,7 @@ SChannel::connectionThread(
&payloadLength))
{
// Procced based on the packet type
switch (ChannelProto::getPktType(*reqDataPktHdr))
switch (ChannelProto::getPktType(*reqDataPktHdr, sizeof(reqDataPktHdr)))
{
case ChannelProto::ReqDataCarrierPacketType:

View File

@@ -101,13 +101,13 @@ pthread_mutex_t interlockedMutex;
typedef map<int32_t, ServerReq*> RSMap;
typedef RSMap::iterator RSMapIter;
typedef pair<RSMapIter, bool> RSIterBoolPair;
RSMap rsMap;
RSMap rsMap;
int numActiveRequests = 0;
//
// Next request id (Can not be zero)
//
int32_t nextReqId = 1;
uint32_t nextReqId = 1;
//
// Pending ServerRequests List and count - Server requests are staged on this lists until
@@ -436,15 +436,25 @@ BindSocket(int socketToBind)
// Remove pre-existing socket
unlink(listenSocketFile);
// Setup the address that the daemon will use to listen
// for connections.
listenAddr.sun_family = AF_UNIX;
strcpy(listenAddr.sun_path, listenSocketFile);
// Verify that the specified path is not too long
if (strlen(listenSocketFile) < sizeof(sizeof(listenAddr.sun_path)))
{
// Setup the address that the daemon will use to listen
// for connections.
listenAddr.sun_family = AF_UNIX;
strncpy(listenAddr.sun_path, listenSocketFile, sizeof(listenAddr.sun_path) - 1);
// Perform the bind operation
retStatus = bind(socketToBind,
(const sockaddr*) &listenAddr,
sizeof(listenAddr.sun_family) + strlen(listenAddr.sun_path));
// Perform the bind operation
retStatus = bind(socketToBind,
(const sockaddr*) &listenAddr,
sizeof(listenAddr.sun_family) + strlen(listenAddr.sun_path));
}
else
{
DbgTrace(0, "BindSocket- Listen socket file path too long\n", 0);
errno = ERANGE;
retStatus = -1;
}
// Return the file creation mask to its previous value
umask(prevMask);
@@ -688,7 +698,7 @@ void* ServiceConnectionsThread(void)
//++=======================================================================
extern "C"
int32_t
uint32_t
IpcServerGetRequest(void)
//
// Arguments In: None.
@@ -813,7 +823,7 @@ exit:
extern "C"
int32_t
IpcServerGetRequestData(
IN int32_t requestId,
IN uint32_t requestId,
INOUT char **ppReqData)
//
// Arguments In: requestId - The id of the request being processed.
@@ -884,7 +894,7 @@ IpcServerGetRequestData(
extern "C"
void
IpcServerCompleteRequest(
IN int32_t requestId,
IN uint32_t requestId,
IN char *pReplyData)
//
// Arguments In: requestId - The id of the request being completed.
@@ -950,7 +960,7 @@ IpcServerCompleteRequest(
extern "C"
void
IpcServerAbortRequest(
IN int32_t requestId)
IN uint32_t requestId)
//
// Arguments In: requestId - The id of the request being aborted.
//
@@ -1246,7 +1256,7 @@ IpcServerInit(
DbgTrace(1, "IpcServerInit- Start\n", 0);
// Check input parameters
if (pAppName == NULL)
if (pName == NULL)
{
DbgTrace(0, "IpcServerInit- Invalid parameter\n", 0);
goto exit;