From 409fc825e504f1aa592c1ad4d0d12a2029423c40 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgallagh@redhat.com>
Date: Tue, 8 Jun 2010 15:47:34 -0400
Subject: [PATCH] Properly handle read() and write() throughout the SSSD

We need to guarantee at all times that reads and writes complete
successfully. This means that they must be checked for returning
EINTR and EAGAIN, and all writes must be wrapped in a loop to
ensure that they do not truncate their output.
---
 src/krb5_plugin/sssd_krb5_locator_plugin.c |    2 +-
 src/monitor/monitor.c                      |    4 +-
 src/providers/krb5/krb5_common.c           |   25 +++++++++----
 src/sss_client/common.c                    |   16 ++++++++
 src/sss_client/pam_sss.c                   |    2 +-
 src/tools/files.c                          |   55 ++++++++++++++++++----------
 src/util/backup_file.c                     |    4 +-
 src/util/find_uid.c                        |   13 +++++--
 src/util/server.c                          |   52 +++++++++++++++++++++++---
 9 files changed, 131 insertions(+), 42 deletions(-)

diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c
index dfc9c197ce42d4fd86d84edcfc947202c9a9766b..70479efdc7c32e919871617c446fb3ce5899bbc2 100644
--- a/src/krb5_plugin/sssd_krb5_locator_plugin.c
+++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c
@@ -133,7 +133,7 @@ static int get_krb5info(const char *realm, struct sssd_ctx *ctx,
     memset(buf, 0, BUFSIZE+1);
     while (len != 0 && (ret = read(fd, p, len)) != 0) {
         if (ret == -1) {
-            if (errno == EINTR) continue;
+            if (errno == EINTR || errno == EAGAIN) continue;
             PLUGIN_DEBUG(("read failed [%d][%s].\n", errno, strerror(errno)));
             close(fd);
             goto done;
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 75d893ebf16db25999d61758139c57bb5080ebaf..acac12dba4eafdf32902de0c50056f3b0a82924e 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -1372,7 +1372,7 @@ static void process_config_file(struct tevent_context *ev,
         len = read(file_ctx->mt_ctx->inotify_fd, buf+total_len,
                    event_size-total_len);
         if (len == -1) {
-            if (errno == EINTR) continue;
+            if (errno == EINTR || errno == EAGAIN) continue;
             DEBUG(0, ("Critical error reading inotify file descriptor.\n"));
             goto done;
         }
@@ -1393,7 +1393,7 @@ static void process_config_file(struct tevent_context *ev,
         while (total_len < in_event->len) {
             len = read(file_ctx->mt_ctx->inotify_fd, &name, in_event->len);
             if (len == -1) {
-                if (errno == EINTR) continue;
+                if (errno == EINTR || errno == EAGAIN) continue;
                 DEBUG(0, ("Critical error reading inotify file descriptor.\n"));
                 goto done;
             }
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index c78f0e60ca51ed365c5f5011366102d37cee2814..fbc308895f37bc71e14e0ab7137448a01f0a9bd1 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -158,6 +158,7 @@ errno_t write_krb5info_file(const char *realm, const char *server,
     TALLOC_CTX *tmp_ctx = NULL;
     const char *name_tmpl = NULL;
     int server_len;
+    ssize_t written;
 
     if (realm == NULL || *realm == '\0' || server == NULL || *server == '\0' ||
         service == NULL || service == '\0') {
@@ -203,14 +204,24 @@ errno_t write_krb5info_file(const char *realm, const char *server,
         goto done;
     }
 
-    ret = write(fd, server, server_len);
-    if (ret == -1) {
-        DEBUG(1, ("write failed [%d][%s].\n", errno, strerror(errno)));
-        goto done;
+    written = 0;
+    while (written < server_len) {
+        ret = write(fd, server+written, server_len-written);
+        if (ret == -1) {
+            if (errno == EINTR || errno == EAGAIN) {
+                continue;
+            }
+            DEBUG(1, ("write failed [%d][%s].\n", errno, strerror(errno)));
+            goto done;
+        }
+        else {
+            written += ret;
+        }
     }
-    if (ret != server_len) {
-        DEBUG(1, ("Partial write occured, this should never happen.\n"));
-        ret = EINTR;
+
+    if (written != server_len) {
+        DEBUG(1, ("Write error, wrote [%d] bytes, expected [%d]\n",
+                   written, server_len));
         goto done;
     }
 
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 237b90aba0a6af74bbc2d5b710e221768541f135..a4856e0884e7151c7aaeaf9f40cae9e767a8a8e9 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -123,6 +123,7 @@ static enum nss_status sss_nss_send_req(enum sss_cli_command cmd,
             return NSS_STATUS_UNAVAIL;
         }
 
+        errno = 0;
         if (datasent < SSS_NSS_HEADER_SIZE) {
             res = write(sss_cli_sd,
                         (char *)header + datasent,
@@ -133,8 +134,15 @@ static enum nss_status sss_nss_send_req(enum sss_cli_command cmd,
                         (const char *)rd->data + rdsent,
                         rd->len - rdsent);
         }
+        error = errno;
 
         if ((res == -1) || (res == 0)) {
+            if ((error == EINTR) || error == EAGAIN) {
+                /* If the write was interrupted, go back through
+                 * the loop and try again
+                 */
+                continue;
+            }
 
             /* Write failed */
             sss_cli_close_socket();
@@ -217,6 +225,7 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
             return NSS_STATUS_UNAVAIL;
         }
 
+        errno = 0;
         if (datarecv < SSS_NSS_HEADER_SIZE) {
             res = read(sss_cli_sd,
                        (char *)header + datarecv,
@@ -227,8 +236,15 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
                        (char *)(*buf) + bufrecv,
                        header[0] - datarecv);
         }
+        error = errno;
 
         if ((res == -1) || (res == 0)) {
+            if ((error == EINTR) || error == EAGAIN) {
+                /* If the read was interrupted, go back through
+                 * the loop and try again
+                 */
+                continue;
+            }
 
             /* Read failed.  I think the only useful thing
              * we can do here is just return -1 and fail
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index d2758004fe094ddd8cc8d34c8165a07cdc694be4..d7a6bb7689f5014b7c2dcaf60f5ae15a4f5123e1 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -487,7 +487,7 @@ static errno_t display_pw_reset_message(pam_handle_t *pamh,
     while (total_len < stat_buf.st_size) {
         ret = read(fd, msg_buf + total_len, stat_buf.st_size - total_len);
         if (ret == -1) {
-            if (errno == EINTR) continue;
+            if (errno == EINTR || errno == EAGAIN) continue;
             ret = errno;
             D(("read failed [%d][%s].", ret, strerror(ret)));
             goto done;
diff --git a/src/tools/files.c b/src/tools/files.c
index b3b516ea466565af6977d4b07bf0cbc877f891ce..27ebf72dbb7ede7da0f1d7229d0063eac7b0a2dc 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -402,7 +402,7 @@ static int copy_file(const char *src,
     int ifd = -1;
     int ofd = -1;
     char buf[1024];
-    ssize_t cnt, written, offset;
+    ssize_t cnt, written, res;
     struct stat fstatbuf;
 
     ifd = open(src, O_RDONLY);
@@ -454,27 +454,44 @@ static int copy_file(const char *src,
         goto fail;
     }
 
-    while ((cnt = read(ifd, buf, sizeof(buf))) > 0) {
-        offset = 0;
-        while (cnt > 0) {
-            written = write(ofd, buf+offset, (size_t)cnt);
-            if (written == -1) {
-                ret = errno;
-                DEBUG(1, ("Cannot write() to source file '%s': [%d][%s].\n",
-                            dst, ret, strerror(ret)));
-                goto fail;
+    while ((cnt = read(ifd, buf, sizeof(buf))) != 0) {
+        if (cnt == -1) {
+            if (errno == EINTR || errno == EAGAIN) {
+                continue;
             }
-            offset += written;
-            cnt -= written;
+
+            DEBUG(1, ("Cannot read() from source file '%s': [%d][%s].\n",
+                        src, ret, strerror(ret)));
+            goto fail;
         }
-    }
-    if (cnt == -1) {
-        ret = errno;
-        DEBUG(1, ("Cannot read() from source file '%s': [%d][%s].\n",
-                    dst, ret, strerror(ret)));
-        goto fail;
-    }
+        else if (cnt > 0) {
+            /* Copy the buffer to the new file */
+            written = 0;
+            while (written < cnt) {
+                res = write(ofd, buf+written, (size_t)cnt-written);
+                if (res == -1) {
+                    ret = errno;
+                    if (ret == EINTR || ret == EAGAIN) {
+                        /* retry the write */
+                        continue;
+                    }
+                    DEBUG(1, ("Cannot write() to destination file '%s': [%d][%s].\n",
+                                dst, ret, strerror(ret)));
+                    goto fail;
+                }
+                else if (res <= 0) {
+                    DEBUG(1, ("Unexpected result from write(): [%d]\n", res));
+                    goto fail;
+                }
 
+                written += res;
+            }
+        }
+        else {
+            DEBUG(1, ("Unexpected return code of read [%d]\n", cnt));
+            goto fail;
+        }
+    }
 
     ret = close(ifd);
     ifd = -1;
diff --git a/src/util/backup_file.c b/src/util/backup_file.c
index cf9ddf3034c531954beaae69ca49c89c1efa233c..990793278901c04291eab44056fa046b40aa4fad 100644
--- a/src/util/backup_file.c
+++ b/src/util/backup_file.c
@@ -86,7 +86,7 @@ int backup_file(const char *src_file, int dbglvl)
     while (1) {
         num = read(src_fd, buf, BUFFER_SIZE);
         if (num < 0) {
-            if (errno == EINTR) continue;
+            if (errno == EINTR || errno == EAGAIN) continue;
             ret = errno;
             DEBUG(dbglvl, ("Error (%d [%s]) reading from source %s\n",
                            ret, strerror(ret), src_file));
@@ -101,7 +101,7 @@ int backup_file(const char *src_file, int dbglvl)
             errno = 0;
             num = write(dst_fd, &buf[pos], count);
             if (num < 0) {
-                if (errno == EINTR) continue;
+                if (errno == EINTR || errno == EAGAIN) continue;
                 ret = errno;
                 DEBUG(dbglvl, ("Error (%d [%s]) writing to destination %s\n",
                                ret, strerror(ret), dst_file));
diff --git a/src/util/find_uid.c b/src/util/find_uid.c
index 965966ef377318fc8551f487cbb83de638168978..952aeea4abd932790e07f36ecd30a0b8a83e45b7 100644
--- a/src/util/find_uid.c
+++ b/src/util/find_uid.c
@@ -100,10 +100,15 @@ static errno_t get_uid_from_pid(const pid_t pid, uid_t *uid)
         DEBUG(1, ("open failed [%d][%s].\n", errno, strerror(errno)));
         return errno;
     }
-    ret = read(fd, buf, BUFSIZE);
-    if (ret == -1) {
-        DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
-        return errno;
+
+    while ((ret = read(fd, buf, BUFSIZE)) != 0) {
+        if (ret == -1) {
+            if (errno == EINTR || errno == EAGAIN) {
+                continue;
+            }
+            DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
+            return errno;
+        }
     }
 
     ret = close(fd);
diff --git a/src/util/server.c b/src/util/server.c
index 007b362414cac34412ba29b8bae76bb0df9b7a7c..8f5b4414616b0804384097dec539a3a7f6ece626 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -104,6 +104,10 @@ int pidfile(const char *path, const char *name)
     char *file;
     int fd;
     int ret, err;
+    ssize_t len;
+    ssize_t size;
+    ssize_t written;
+    ssize_t pidlen = sizeof(pid_str - 1);
 
     file = talloc_asprintf(NULL, "%s/%s.pid", path, name);
     if (!file) {
@@ -114,9 +118,30 @@ int pidfile(const char *path, const char *name)
     err = errno;
     if (fd != -1) {
 
-        pid_str[sizeof(pid_str) -1] = '\0';
-        ret = read(fd, pid_str, sizeof(pid_str) -1);
-        if (ret > 0) {
+        pid_str[pidlen] = '\0';
+
+
+        while ((ret = read(fd, pid_str + len, pidlen - len)) != 0) {
+            if (ret == -1) {
+                if (errno == EINTR || errno == EAGAIN) {
+                    continue;
+                }
+                DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
+                break;
+            } else if (ret > 0) {
+                len += ret;
+                if (len > pidlen) {
+                    DEBUG(1, ("read too much, this should never happen.\n"));
+                    break;
+                }
+                continue;
+            } else {
+                DEBUG(1, ("unexpected return code of read [%d].\n", ret));
+                break;
+            }
+        }
+
+        if (ret == 0) {
             /* let's check the pid */
 
             pid = (pid_t)atoi(pid_str);
@@ -159,10 +184,25 @@ int pidfile(const char *path, const char *name)
 
     memset(pid_str, 0, sizeof(pid_str));
     snprintf(pid_str, sizeof(pid_str) -1, "%u\n", (unsigned int) getpid());
+    size = strlen(pid_str);
 
-    ret = write(fd, pid_str, strlen(pid_str));
-    err = errno;
-    if (ret != strlen(pid_str)) {
+    written = 0;
+    while (written < size) {
+        ret = write(fd, pid_str+written, size-written);
+        if (ret == -1) {
+            err = errno;
+            if (err == EINTR || err == EAGAIN) {
+                continue;
+            }
+            DEBUG(1, ("write failed [%d][%s]\n", err, strerror(err)));
+            break;
+        }
+        else {
+            written += ret;
+        }
+    }
+
+    if (written != size) {
         close(fd);
         return err;
     }
-- 
1.7.0.1

