[SSSD] [PATCH] fix possible short reads in kerberos provider

Sumit Bose sbose at redhat.com
Wed Sep 23 11:57:39 UTC 2009


Hi,

this patch should fix #179.

bye,
Sumit
-------------- next part --------------
>From 3924dc5b18c9efbf3dbe9a18888888e7e79bcbff Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 23 Sep 2009 13:54:49 +0200
Subject: [PATCH] fix possible short reads in kerberos provider

---
 server/providers/krb5/krb5_auth.c  |   23 +++++++++++++++++----
 server/providers/krb5/krb5_child.c |   38 ++++++++++++++++++++++++++---------
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/server/providers/krb5/krb5_auth.c b/server/providers/krb5/krb5_auth.c
index 0eac91f..1a5bb8b 100644
--- a/server/providers/krb5/krb5_auth.c
+++ b/server/providers/krb5/krb5_auth.c
@@ -411,6 +411,7 @@ static struct tevent_req *read_pipe_send(TALLOC_CTX *memctx,
 
     state->fd = fd;
     state->buf = talloc_array(state, uint8_t, MAX_CHILD_MSG_SIZE);
+    state->len = 0;
     if (state->buf == NULL) goto fail;
 
     fde = tevent_add_fd(ev, state, fd, TEVENT_FD_READ,
@@ -435,22 +436,34 @@ static void read_pipe_done(struct tevent_context *ev, struct tevent_fd *fde,
     struct read_pipe_state *state = tevent_req_data(req, struct read_pipe_state);
 
     if (flags & TEVENT_FD_WRITE) {
-        DEBUG(1, ("client_response_handler called with TEVENT_FD_WRITE, this should not happen.\n"));
+        DEBUG(1, ("read_pipe_done called with TEVENT_FD_WRITE, this should not happen.\n"));
         tevent_req_error(req, EINVAL);
         return;
     }
 
-    size = read(state->fd, state->buf, talloc_get_size(state->buf));
+    size = read(state->fd, state->buf + state->len, talloc_get_size(state->buf) - state->len);
     if (size == -1) {
         if (errno == EAGAIN || errno == EINTR) return;
         DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
         tevent_req_error(req, errno);
         return;
+    } else if (size > 0) {
+        state->len += size;
+        if (state->len > talloc_get_size(state->buf)) {
+            DEBUG(1, ("read to much, this should never happen.\n"));
+            tevent_req_error(req, EINVAL);
+            return;
+        }
+        return;
+    } else if (size == 0) {
+        tevent_req_done(req);
+        return;
+    } else {
+        DEBUG(1, ("unexpected return value of read [%d].\n", size));
+        tevent_req_error(req, EINVAL);
+        return;
     }
-    state->len = size;
 
-    tevent_req_done(req);
-    return;
 }
 
 static ssize_t read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
diff --git a/server/providers/krb5/krb5_child.c b/server/providers/krb5/krb5_child.c
index 3673c74..70844d5 100644
--- a/server/providers/krb5/krb5_child.c
+++ b/server/providers/krb5/krb5_child.c
@@ -34,6 +34,8 @@
 #include "providers/krb5/krb5_auth.h"
 #include "providers/krb5/krb5_utils.h"
 
+#define IN_BUF_SIZE 512
+
 struct krb5_req {
     krb5_context ctx;
     krb5_ccache cc;
@@ -571,6 +573,7 @@ int main(int argc, char *argv[])
 {
     uint8_t *buf = NULL;
     int ret;
+    ssize_t len = 0;
     struct pam_data *pd = NULL;
     struct krb5_req *kr = NULL;
     char *ccname;
@@ -579,32 +582,43 @@ int main(int argc, char *argv[])
 
     pd = talloc(NULL, struct pam_data);
 
-    buf = talloc_size(pd, sizeof(uint8_t)*512);
+    buf = talloc_size(pd, sizeof(uint8_t)*IN_BUF_SIZE);
     if (buf == NULL) {
         DEBUG(1, ("malloc failed.\n"));
         _exit(-1);
     }
 
-    ret = read(STDIN_FILENO, buf, 512);
-    if (ret == -1) {
-        DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
-        talloc_free(pd);
-        exit(-1);
+    while ((ret = read(STDIN_FILENO, buf + len, IN_BUF_SIZE - len)) != 0) {
+        if (ret == -1) {
+            if (errno == EINTR || errno == EAGAIN) {
+                continue;
+            }
+            DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
+            goto fail;
+        } else if (ret > 0) {
+            len += ret;
+            if (len > IN_BUF_SIZE) {
+                DEBUG(1, ("read too much, this should never happen.\n"));
+                goto fail;
+            }
+            continue;
+        } else {
+            DEBUG(1, ("unexpected return code of read [%d].\n", ret));
+            goto fail;
+        }
     }
     close(STDIN_FILENO);
 
     ret = unpack_buffer(buf, ret, pd, &ccname);
     if (ret != EOK) {
         DEBUG(1, ("unpack_buffer failed.\n"));
-        talloc_free(pd);
-        exit(-1);
+        goto fail;
     }
 
     ret = krb5_setup(pd, pd->upn, &kr);
     if (ret != EOK) {
         DEBUG(1, ("krb5_setup failed.\n"));
-        talloc_free(pd);
-        exit(-1);
+        goto fail;
     }
     kr->ccname = ccname;
 
@@ -617,4 +631,8 @@ int main(int argc, char *argv[])
     talloc_free(pd);
 
     return 0;
+
+fail:
+    talloc_free(pd);
+    exit(-1);
 }
-- 
1.6.2.5



More information about the sssd-devel mailing list