fix SendCall memory leaks

cmd/pcloudcc/control_tools.cpp

@@ -59,8 +59,9 @@ enum command_ids_ {
 int start_crypto(const char *pass) {
   int ret;
   char *errm;
+  size_t errm_size;
 
-  if (SendCall(STARTCRYPTO, pass, &ret, &errm)) {
+  if (SendCall(STARTCRYPTO, pass, &ret, &errm, &errm_size)) {
     std::cout << "Start Crypto failed. return is " << ret << " and message is "
               << errm << std::endl;
   } else {
@@ -74,8 +75,9 @@ int start_crypto(const char *pass) {
 int stop_crypto() {
   int ret;
   char *errm;
+  size_t errm_size;
 
-  if (SendCall(STOPCRYPTO, "", &ret, &errm)) {
+  if (SendCall(STOPCRYPTO, "", &ret, &errm, &errm_size)) {
     std::cout << "Stop Crypto failed. return is " << ret << " and message is "
               << errm << std::endl;
   } else {
@@ -89,8 +91,9 @@ int stop_crypto() {
 int finalize() {
   int ret;
   char *errm;
+  size_t errm_size;
 
-  if (SendCall(FINALIZE, "", &ret, &errm)) {
+  if (SendCall(FINALIZE, "", &ret, &errm, &errm_size)) {
     std::cout << "Finalize failed. return is " << ret << " and message is "
               << errm << std::endl;
   } else {

lib/poverlay_linux/overlay_client.c

@@ -62,7 +62,9 @@ const char *clsoc = "/tmp/pcloud_unix_soc.sock";
 int QueryState(pCloud_FileState *state, char *path) {
   int rep = 0;
   char *errm;
-  if (!SendCall(4, path /*IN*/, &rep, &errm)) {
+  size_t errm_size;
+
+  if (!SendCall(4, path /*IN*/, &rep, &errm, &errm_size)) {
     debug(D_NOTICE, "QueryState responese rep[%d] path[%s]", rep, path);
     if (errm)
       debug(D_NOTICE, "The error is %s", errm);
@@ -81,73 +83,153 @@ int QueryState(pCloud_FileState *state, char *path) {
 }
 
 int SendCall(int id /*IN*/, const char *path /*IN*/, int *ret /*OUT*/,
-             char **out /*OUT*/) {
+             char **out /*OUT*/, size_t *out_size) {
   struct sockaddr_un addr;
 
-  int fd, rc;
-  int path_size = strlen(path);
-  int mess_size = sizeof(message) + path_size + 1;
-  int bytes_writen = 0;
+  int result, sendbytes, rc;
+  int fd = -1;
+  int sendlen = strlen(path);
+  int sendsize = sizeof(message) + sendlen + 1;
+
   char *curbuf = NULL;
-  char *buf = NULL;
-  uint32_t bufflen = 0;
-  char sendbuf[mess_size];
-  int bytes_read = 0;
+  char sendbuf[sendsize];
+
+  char *recvbuf = NULL;
+  size_t recvsize, recvbytes;
+  size_t recvchunk = 32; // read response in 32-byte chunks
+
   message *rep = NULL;
+  const char *error_msg;
 
   debug(D_NOTICE, "SendCall id[%d] path[%s]\n", id, path);
 
+  // prepare socket fd
   if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
-    *out = strndup("Unable to create UNIX socket", 27);
+    error_msg = "Unable to create unix socket";
+    *out = strdup(error_msg);
+    if (*out == NULL) {
+      debug(D_ERROR,
+            "on socket(): failed to allocate memory for output message");
+      *ret = -255;
+      result = -255;
+      goto cleanup;
+    }
+    *out_size = strlen(error_msg) + 1;
     *ret = -3;
-    return -3;
+    result = -3;
+    goto cleanup;
   }
+
+  // connect to socket
   memset(&addr, 0, sizeof(addr));
   addr.sun_family = AF_UNIX;
   strncpy(addr.sun_path, clsoc, sizeof(addr.sun_path) - 1);
-
   if (connect(fd, (struct sockaddr *)&addr, SUN_LEN(&addr)) == -1) {
-    *out = strndup("Unable to connect to UNIX socket", 32);
+    error_msg = "Unable to connect to UNIX socket";
+    *out = strdup(error_msg);
+    if (*out == NULL) {
+      debug(D_ERROR,
+            "on connect(): failed to allocate memory for output message");
+      *ret = -255;
+      result = -255;
+      goto cleanup;
+    }
+    *out_size = strlen(error_msg) + 1;
     *ret = -4;
-    return -4;
+    result = -4;
+    goto cleanup;
   }
 
+  // prepare and send the message to the socket
   message *mes = (message *)sendbuf;
-  memset(mes, 0, mess_size);
+  memset(mes, 0, sendsize);
   mes->type = id;
-  strncpy(mes->value, path, path_size);
-  mes->length = mess_size;
+  strncpy(mes->value, path, sendlen);
+  mes->length = sendsize;
   curbuf = (char *)mes;
-  while ((rc = write(fd, curbuf, (mes->length - bytes_writen))) > 0) {
-    bytes_writen += rc;
+  while ((rc = write(fd, curbuf, (mes->length - sendbytes))) > 0) {
+    sendbytes += rc;
     curbuf = curbuf + rc;
   }
-  debug(D_NOTICE, "QueryState bytes send[%d]\n", bytes_writen);
-  if (bytes_writen != mes->length) {
+  debug(D_NOTICE, "QueryState bytes send[%d]\n", sendbytes);
+  if (sendbytes != mes->length) {
     *out = strndup("Communication error", 19);
-    close(fd);
+    error_msg = "Communication error";
+    *out = strdup(error_msg);
+    if (*out == NULL) {
+      debug(D_ERROR, "while checking bytes_written: failed to allocate memory "
+                     "for output message");
+      *ret = -255;
+      result = -255;
+      goto cleanup;
+    }
+    *out_size = strlen(error_msg) + 1;
     *ret = -5;
-    return -5;
+    result = -5;
+    goto cleanup;
   }
 
-  bufflen = read_x_bytes(fd, 4, buf);
+  // read the response from the socket
+  rc = 0;
+  for (;;) {
+    recvbuf = realloc(recvbuf, recvsize + recvchunk);
+    if (!recvbuf) {
+      debug(D_ERROR, "failed to allocate memory to response buffer");
+      *ret = -255;
+      result = -255;
+      goto cleanup;
+    }
 
-  if (bufflen <= 0) {
-    debug(D_NOTICE, "Message size could not be read![%d]\n", bufflen);
-    return -6;
+    rc = read(fd, recvbuf + recvsize, recvchunk);
+    if (rc < 0) {
+      debug(D_ERROR, "failed to read from socket into response buffer");
+      *ret = -255;
+      result = -255;
+      goto cleanup;
+    }
+    if (rc == 0) {
+      break; // end of response
+    }
+    recvsize += rc;
+    recvbytes += rc;
+
+    if (recvbytes >= sizeof(message)) {
+      message *rep = (message *)recvbuf;
+      if (recvbytes >= rep->length) {
+        break; // entire message read
+      }
+    }
   }
-  buf = (char *)malloc(bufflen);
-  rep = (message *)buf;
-  rep->length = bufflen;
 
-  read_x_bytes(fd, bufflen - 4, buf + 4);
+  if (recvbytes < sizeof(message)) {
+    debug(D_ERROR, "got incomplete message from socket");
+    *ret = -255;
+    result = -255;
+    goto cleanup;
+  }
 
+  rep = (message *)recvbuf;
   *ret = rep->type;
-  *out = strndup(rep->value, rep->length - sizeof(message));
 
-  close(fd);
+  size_t value_size = rep->length - sizeof(message);
+  *out = malloc(value_size + 1);
+  if (!*out) {
+    debug(D_ERROR, "failed to allocate memory to output buffer");
+    *ret = -255;
+    result = -255;
+    goto cleanup;
+  }
+  memcpy(*out, rep->value, value_size);
+  (*out)[value_size] = '\0';
+  *out_size = value_size + 1;
 
-  return 0;
+cleanup:
+  if (fd != -1)
+    close(fd);
+  if (recvbuf)
+    free(recvbuf);
+
+  return result;
 }
 
 #ifdef PCLOUD_TESTING
@@ -155,6 +237,7 @@ int main(int arc, char **argv) {
   int i, j = 0;
   pCloud_FileState state;
   char *errm;
+  size_t errm_size;
 
   for (i = 1; i < arc; ++i) {
     QueryState(&state, argv[i]);
@@ -168,14 +251,13 @@ int main(int arc, char **argv) {
       printf("File %s FileStateInvalid\n", argv[i]);
     else
       printf("Not valid state returned for file %s\n", argv[i]);
-    SendCall(20, argv[i], &j, &errm);
+    SendCall(20, argv[i], &j, &errm, &errm_size);
     printf("Call 20 returned %d msg %s \n", j, errm);
-    SendCall(21, argv[i], &j, &errm);
+    SendCall(21, argv[i], &j, &errm, &errm_size);
     printf("Call 21 returned %d msg %s \n", j, errm);
-    SendCall(22, argv[i], &j, &errm);
+    SendCall(22, argv[i], &j, &errm, &errm_size);
     printf("Call 22 returned %d msg %s \n", j, errm);
-
-    SendCall(23, argv[i], &j, &errm);
+    SendCall(23, argv[i], &j, &errm, &errm_size);
     printf("Call 22 returned %d msg %s \n", j, errm);
   }
   return 0;

lib/poverlay_linux/overlay_client.h

@@ -26,6 +26,11 @@
   DAMAGE.
 */
 
+/*
+Dependencies:
+  - stddef.h
+ */
+
 #ifndef OVERLAY_CLIENT_H
 #define OVERLAY_CLIENT_H
 
@@ -44,7 +49,7 @@ typedef enum _pCloud_FileState {
 int QueryState(pCloud_FileState *state /*OUT*/, char *path /*IN*/);
 
 int SendCall(int id /*IN*/, const char *path /*IN*/, int *ret /*OUT*/,
-             char **out /*OUT*/);
+             char **out /*OUT*/, size_t *out_size);
 #ifdef __cplusplus
 }
 #endif