>From f5055945580d7f1fd8eac019c610801e3b0e7068 Mon Sep 17 00:00:00 2001
From: Simo Sorce <ssorce@redhat.com>
Date: Mon, 1 Mar 2010 16:41:12 -0500
Subject: [PATCH] Improve safe alignment buffer handling macros

Make the counter optional so that alignment safe macros can be used also where
there is no counter to update.
Change arguments names so that they are not deceiving (ptr normlly identify a
pointer)
Turn the memcpy substitute into an inline function so that passing a pointer to
rp and checking for it doesn't make the compiler spit lots of warnings.
---
 src/providers/krb5/krb5_auth.c          |   33 +++++++++++++------------
 src/providers/krb5/krb5_child.c         |   30 ++++++++++++------------
 src/providers/ldap/ldap_child.c         |   14 +++++-----
 src/providers/ldap/sdap_child_helpers.c |   22 ++++++++--------
 src/util/util.h                         |   39 +++++++++++++++++++------------
 5 files changed, 74 insertions(+), 64 deletions(-)

diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 0e5230c..e46ee2b 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -331,28 +331,29 @@ errno_t create_send_buffer(struct krb5child_req *kr, struct io_buffer **io_buf)
     }
 
     rp = 0;
-    COPY_UINT32(&buf->data[rp], &kr->pd->cmd, rp);
-    COPY_UINT32(&buf->data[rp], &kr->uid, rp);
-    COPY_UINT32(&buf->data[rp], &kr->gid, rp);
-    COPY_UINT32(&buf->data[rp], &validate, rp);
-    COPY_UINT32(&buf->data[rp], &kr->is_offline, rp);
+    SAFEALIGN_COPY_UINT32(&buf->data[rp], &kr->pd->cmd, &rp);
+    SAFEALIGN_COPY_UINT32(&buf->data[rp], &kr->uid, &rp);
+    SAFEALIGN_COPY_UINT32(&buf->data[rp], &kr->gid, &rp);
+    SAFEALIGN_COPY_UINT32(&buf->data[rp], &validate, &rp);
+    SAFEALIGN_COPY_UINT32(&buf->data[rp], &kr->is_offline, &rp);
 
-    COPY_UINT32_VALUE(&buf->data[rp], strlen(kr->upn), rp);
-    COPY_MEM(&buf->data[rp], kr->upn, rp, strlen(kr->upn));
+    SAFEALIGN_SET_UINT32(&buf->data[rp], strlen(kr->upn), &rp);
+    safealign_memcpy(&buf->data[rp], kr->upn, strlen(kr->upn), &rp);
 
-    COPY_UINT32_VALUE(&buf->data[rp], strlen(kr->ccname), rp);
-    COPY_MEM(&buf->data[rp], kr->ccname, rp, strlen(kr->ccname));
+    SAFEALIGN_SET_UINT32(&buf->data[rp], strlen(kr->ccname), &rp);
+    safealign_memcpy(&buf->data[rp], kr->ccname, strlen(kr->ccname), &rp);
 
-    COPY_UINT32_VALUE(&buf->data[rp], strlen(keytab), rp);
-    COPY_MEM(&buf->data[rp], keytab, rp, strlen(keytab));
+    SAFEALIGN_SET_UINT32(&buf->data[rp], strlen(keytab), &rp);
+    safealign_memcpy(&buf->data[rp], keytab, strlen(keytab), &rp);
 
-    COPY_UINT32(&buf->data[rp], &kr->pd->authtok_size, rp);
-    COPY_MEM(&buf->data[rp], kr->pd->authtok, rp, kr->pd->authtok_size);
+    SAFEALIGN_COPY_UINT32(&buf->data[rp], &kr->pd->authtok_size, &rp);
+    safealign_memcpy(&buf->data[rp], kr->pd->authtok,
+                     kr->pd->authtok_size, &rp);
 
     if (kr->pd->cmd == SSS_PAM_CHAUTHTOK) {
-        COPY_UINT32(&buf->data[rp], &kr->pd->newauthtok_size, rp);
-        COPY_MEM(&buf->data[rp], kr->pd->newauthtok,
-                 rp, kr->pd->newauthtok_size);
+        SAFEALIGN_COPY_UINT32(&buf->data[rp], &kr->pd->newauthtok_size, &rp);
+        safealign_memcpy(&buf->data[rp], kr->pd->newauthtok,
+                         kr->pd->newauthtok_size, &rp);
     }
 
     *io_buf = buf;
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 08df598..234b838 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -264,17 +264,17 @@ static struct response *init_response(TALLOC_CTX *mem_ctx) {
 static errno_t pack_response_packet(struct response *resp, int status, int type,
                                     size_t len, const uint8_t *data)
 {
-    int p=0;
+    size_t p = 0;
 
     if ((3*sizeof(int32_t) + len +1) > resp->max_size) {
         DEBUG(1, ("response message too big.\n"));
         return ENOMEM;
     }
 
-    COPY_INT32_VALUE(&resp->buf[p], status, p);
-    COPY_INT32_VALUE(&resp->buf[p], type, p);
-    COPY_INT32_VALUE(&resp->buf[p], len, p);
-    COPY_MEM(&resp->buf[p], data, p, len);
+    SAFEALIGN_SET_INT32(&resp->buf[p], status, &p);
+    SAFEALIGN_SET_INT32(&resp->buf[p], type, &p);
+    SAFEALIGN_SET_INT32(&resp->buf[p], len, &p);
+    safealign_memcpy(&resp->buf[p], data, len, &p);
 
     resp->size = p;
 
@@ -733,32 +733,32 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct pam_data *pd,
     uint32_t len;
     uint32_t validate;
 
-    COPY_UINT32_CHECK(&pd->cmd, buf + p, p, size);
-    COPY_UINT32_CHECK(&kr->uid, buf + p, p, size);
-    COPY_UINT32_CHECK(&kr->gid, buf + p, p, size);
-    COPY_UINT32_CHECK(&validate, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&pd->cmd, buf + p, size, &p);
+    SAFEALIGN_COPY_UINT32_CHECK(&kr->uid, buf + p, size, &p);
+    SAFEALIGN_COPY_UINT32_CHECK(&kr->gid, buf + p, size, &p);
+    SAFEALIGN_COPY_UINT32_CHECK(&validate, buf + p, size, &p);
     kr->validate = (validate == 0) ? false : true;
-    COPY_UINT32_CHECK(offline, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(offline, buf + p, size, &p);
 
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
     if ((p + len ) > size) return EINVAL;
     kr->upn = talloc_strndup(pd, (char *)(buf + p), len);
     if (kr->upn == NULL) return ENOMEM;
     p += len;
 
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
     if ((p + len ) > size) return EINVAL;
     kr->ccname = talloc_strndup(pd, (char *)(buf + p), len);
     if (kr->ccname == NULL) return ENOMEM;
     p += len;
 
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
     if ((p + len ) > size) return EINVAL;
     kr->keytab = talloc_strndup(pd, (char *)(buf + p), len);
     if (kr->keytab == NULL) return ENOMEM;
     p += len;
 
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
     if ((p + len) > size) return EINVAL;
     pd->authtok = (uint8_t *)talloc_strndup(pd, (char *)(buf + p), len);
     if (pd->authtok == NULL) return ENOMEM;
@@ -766,7 +766,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct pam_data *pd,
     p += len;
 
     if (pd->cmd == SSS_PAM_CHAUTHTOK) {
-        COPY_UINT32_CHECK(&len, buf + p, p, size);
+        SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
 
         if ((p + len) > size) return EINVAL;
         pd->newauthtok = (uint8_t *)talloc_strndup(pd, (char *)(buf + p), len);
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index 0d34be2..069fbcf 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -51,7 +51,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
     DEBUG(7, ("total buffer size: %d\n", size));
 
     /* realm_str size and length */
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
 
     DEBUG(7, ("realm_str size: %d\n", len));
     if (len) {
@@ -63,7 +63,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
     }
 
     /* princ_str size and length */
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
 
     DEBUG(7, ("princ_str size: %d\n", len));
     if (len) {
@@ -75,7 +75,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
     }
 
     /* keytab_name size and length */
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
 
     DEBUG(7, ("keytab_name size: %d\n", len));
     if (len) {
@@ -92,19 +92,19 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
 static int pack_buffer(struct response *r, int result, const char *msg)
 {
     int len;
-    int p = 0;
+    size_t p = 0;
 
     len = strlen(msg);
     r->size = 2 * sizeof(uint32_t) + len;
 
     /* result */
-    COPY_UINT32_VALUE(&r->buf[p], result, p);
+    SAFEALIGN_SET_UINT32(&r->buf[p], result, &p);
 
     /* message size */
-    COPY_UINT32_VALUE(&r->buf[p], len, p);
+    SAFEALIGN_SET_UINT32(&r->buf[p], len, &p);
 
     /* message itself */
-    COPY_MEM(&r->buf[p], msg, p, len);
+    safealign_memcpy(&r->buf[p], msg, len, &p);
 
     return EOK;
 }
diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index 0a95c8a..273fc67 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/providers/ldap/sdap_child_helpers.c
@@ -166,26 +166,26 @@ static errno_t create_tgt_req_send_buffer(TALLOC_CTX *mem_ctx,
 
     /* realm */
     if (realm_str) {
-        COPY_UINT32_VALUE(&buf->data[rp], strlen(realm_str), rp);
-        COPY_MEM(&buf->data[rp], realm_str, rp, strlen(realm_str));
+        SAFEALIGN_SET_UINT32(&buf->data[rp], strlen(realm_str), &rp);
+        safealign_memcpy(&buf->data[rp], realm_str, strlen(realm_str), &rp);
     } else {
-        COPY_UINT32_VALUE(&buf->data[rp], 0, rp);
+        SAFEALIGN_SET_UINT32(&buf->data[rp], 0, &rp);
     }
 
     /* principal */
     if (princ_str) {
-        COPY_UINT32_VALUE(&buf->data[rp], strlen(princ_str), rp);
-        COPY_MEM(&buf->data[rp], princ_str, rp, strlen(princ_str));
+        SAFEALIGN_SET_UINT32(&buf->data[rp], strlen(princ_str), &rp);
+        safealign_memcpy(&buf->data[rp], princ_str, strlen(princ_str), &rp);
     } else {
-        COPY_UINT32_VALUE(&buf->data[rp], 0, rp);
+        SAFEALIGN_SET_UINT32(&buf->data[rp], 0, &rp);
     }
 
     /* keytab */
     if (keytab_name) {
-        COPY_UINT32_VALUE(&buf->data[rp], strlen(keytab_name), rp);
-        COPY_MEM(&buf->data[rp], keytab_name, rp, strlen(realm_str));
+        SAFEALIGN_SET_UINT32(&buf->data[rp], strlen(keytab_name), &rp);
+        safealign_memcpy(&buf->data[rp], keytab_name, strlen(realm_str), &rp);
     } else {
-        COPY_UINT32_VALUE(&buf->data[rp], 0, rp);
+        SAFEALIGN_SET_UINT32(&buf->data[rp], 0, &rp);
     }
 
     *io_buf = buf;
@@ -202,10 +202,10 @@ static int parse_child_response(TALLOC_CTX *mem_ctx,
     char *ccn;
 
     /* operation result code */
-    COPY_UINT32_CHECK(&res, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&res, buf + p, size, &p);
 
     /* ccache name size */
-    COPY_UINT32_CHECK(&len, buf + p, p, size);
+    SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
 
     if ((p + len ) > size) return EINVAL;
 
diff --git a/src/util/util.h b/src/util/util.h
index 5d2dff2..1f5573d 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -163,26 +163,35 @@ errno_t set_debug_file_from_fd(const int fd);
 #define OUT_OF_ID_RANGE(id, min, max) \
     (id == 0 || (min && (id < min)) || (max && (id > max)))
 
-#define COPY_MEM(to, from, ptr, size) do { \
-    memcpy(to, from, size);                \
-    ptr += size;                           \
+static inline void
+safealign_memcpy(void *dest, const void *src, size_t n, size_t *counter)
+{
+    memcpy(dest, src, n);
+    if (counter) {
+        *counter += n;
+    }
+}
+
+#define SAFEALIGN_SET_VALUE(dest, value, type, pctr) do { \
+    type CV_MACRO_val = (type)(value); \
+    safealign_memcpy(dest, &CV_MACRO_val, sizeof(type), pctr); \
 } while(0)
 
-#define COPY_TYPE(to, from, ptr, type) COPY_MEM(to, from, ptr, sizeof(type))
+#define SAFEALIGN_COPY_UINT32(dest, src, pctr) \
+    safealign_memcpy(dest, src, sizeof(uint32_t), pctr)
 
-#define COPY_VALUE(to, value, ptr, type) do { \
-    type CV_MACRO_val = (type) value;         \
-    COPY_TYPE(to, &CV_MACRO_val, ptr, type);  \
-} while(0)
+#define SAFEALIGN_SET_UINT32(dest, value, pctr) \
+    SAFEALIGN_SET_VALUE(dest, value, uint32_t, pctr)
+
+#define SAFEALIGN_COPY_INT32(dest, src, pctr) \
+    safealign_memcpy(dest, src, sizeof(int32_t), pctr)
 
-#define COPY_UINT32(to, from, ptr) COPY_TYPE(to, from, ptr, uint32_t)
-#define COPY_UINT32_VALUE(to, value, ptr) COPY_VALUE(to, value, ptr, uint32_t)
-#define COPY_INT32(to, from, ptr) COPY_TYPE(to, from, ptr, int32_t)
-#define COPY_INT32_VALUE(to, value, ptr) COPY_VALUE(to, value, ptr, int32_t)
+#define SAFEALIGN_SET_INT32(dest, value, pctr) \
+    SAFEALIGN_SET_VALUE(dest, value, int32_t, pctr)
 
-#define COPY_UINT32_CHECK(to, from, ptr, size) do {     \
-    if ((ptr + sizeof(uint32_t)) > size) return EINVAL; \
-    COPY_UINT32(to, from, ptr);                         \
+#define SAFEALIGN_COPY_UINT32_CHECK(dest, src, len, pctr) do { \
+    if ((*(pctr) + sizeof(uint32_t)) > (len)) return EINVAL; \
+    safealign_memcpy(dest, src, sizeof(uint32_t), pctr); \
 } while(0)
 
 #include "util/dlinklist.h"
-- 
1.6.6.1

