[PATCH] Fix other memory alignment issues

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Similar to George McCollister's patch to the pam code, this patch fixes
other places in the code where we forced data into 32-bit alignment.

Fixes: #390

George was testing KRB authentication with this patch and confirmed[1]
it was working for him on ARM. I was running SSSD with this patch on
x86_64 and did not spot any regressions.

[1] https://fedorahosted.org/sssd/ticket/390#comment:3
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktx1hcACgkQHsardTLnvCUADwCgt9M05Clp45G7IUVLe/sFiJ1n
> ov8AoKSP2QGpoFXvAGGyEbmjfmBlG5sP
> =a2Yy
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Tue, Feb 09, 2010 at 10:39:35PM +0100, Jakub Hrozek wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Similar to George McCollister's patch to the pam code, this patch fixes
>  other places in the code where we forced data into 32-bit alignment.
>  
>  Fixes: #390
>  
>  George was testing KRB authentication with this patch and confirmed[1]
>  it was working for him on ARM. I was running SSSD with this patch on
>  x86_64 and did not spot any regressions.
>  
>  [1] https://fedorahosted.org/sssd/ticket/390#comment:3 
This patch works on i686, too. Please remove the FIXME. The assignment
was introduced in commit 2b4d76971e4ad803b42051fddb3add9fe2215dc2 with
no special reason.

bye,
Sumit

>  -----BEGIN PGP SIGNATURE-----
>  Version: GnuPG v1.4.10 (GNU/Linux)
>  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>  
>  iEYEARECAAYFAktx1hcACgkQHsardTLnvCUADwCgt9M05Clp45G7IUVLe/sFiJ1n
>  ov8AoKSP2QGpoFXvAGGyEbmjfmBlG5sP
>  =a2Yy
>  -----END PGP SIGNATURE-----

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/10/2010 11:55 AM, Sumit Bose wrote:
>  This patch works on i686, too. Please remove the FIXME. The
> assignment
>  was introduced in commit 2b4d76971e4ad803b42051fddb3add9fe2215dc2 with
>  no special reason.
>  
>  bye,
>  Sumit 
attached
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktypK8ACgkQHsardTLnvCXEowCgweCFSuOOlfdpDlVDVu3YsAyt
> 2ZAAn0OOAlRS4rOBpO4kRb1LeLDaJHjC
> =3nt9
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Wed, Feb 10, 2010 at 01:21:04PM +0100, Jakub Hrozek wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 02/10/2010 11:55 AM, Sumit Bose wrote:
>  > This patch works on i686, too. Please remove the FIXME. The assignment
>  > was introduced in commit 2b4d76971e4ad803b42051fddb3add9fe2215dc2 with
>  > no special reason.
>  > 
>  > bye,
>  > Sumit
>  
>  attached 
ACK

bye,
Sumit

> >  -----BEGIN PGP SIGNATURE-----
> >  Version: GnuPG v1.4.10 (GNU/Linux)
> >  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> >  
> >  iEYEARECAAYFAktypK8ACgkQHsardTLnvCXEowCgweCFSuOOlfdpDlVDVu3YsAyt
> >  2ZAAn0OOAlRS4rOBpO4kRb1LeLDaJHjC
> >  =3nt9
> >  -----END PGP SIGNATURE----- 
>  
>  From
> 80541ad346728f5df4fb21760fe4d0f7ca26fee5 Mon Sep 17 00:00:00 2001
>  From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
>  Date: Mon, 25 Jan 2010 23:59:03 +0100
>  Subject: [PATCH] Fix other memory alignment issues
>  
>  Similar to George McCollister's patch to the pam code, this patch fixes
>  other places in the code where we forced data into 32-bit alignment.
>  
>  Fixes: #390
>  ---
>   server/providers/krb5/krb5_auth.c          |   32 +++++++++++++++------------
>   server/providers/krb5/krb5_child.c         |   31 ++++++++++++++------------
>   server/providers/ldap/ldap_child.c         |   13 +++++++----
>   server/providers/ldap/sdap_child_helpers.c |   21 ++++++++++-------
>   4 files changed, 55 insertions(+), 42 deletions(-)
>  
>  diff --git a/server/providers/krb5/krb5_auth.c b/server/providers/krb5/krb5_auth.c
>  index c013ea8..fd0a063 100644
>  --- a/server/providers/krb5/krb5_auth.c
>  +++ b/server/providers/krb5/krb5_auth.c
>  @@ -301,6 +301,7 @@ errno_t create_send_buffer(struct krb5child_req *kr, struct io_buffer
> **io_buf)
>       size_t rp;
>       const char *keytab;
>       uint32_t validate;
>  +    uint32_t c = 0;
>   
>       keytab = dp_opt_get_cstring(kr->krb5_ctx->opts, KRB5_KEYTAB);
>       if (keytab == NULL) {
>  @@ -331,47 +332,50 @@ errno_t create_send_buffer(struct krb5child_req *kr, struct
> io_buffer **io_buf)
>       }
>   
>       rp = 0;
>  -    ((uint32_t *)(&buf->data[rp]))[0] = kr->pd->cmd;
>  +    memcpy(&buf->data[rp], &kr->pd->cmd, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = kr->pd->pw_uid;
>  +    memcpy(&buf->data[rp], &kr->pd->pw_uid, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = kr->pd->gr_gid;
>  +    memcpy(&buf->data[rp], &kr->pd->gr_gid, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = validate;
>  +    memcpy(&buf->data[rp], &validate, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = kr->is_offline;
>  +    memcpy(&buf->data[rp], &kr->is_offline, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = (uint32_t) strlen(kr->pd->upn);
>  +    c = (uint32_t) strlen(kr->pd->upn);
>  +    memcpy(&buf->data[rp], &c, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    memcpy(&buf->data[rp], kr->pd->upn, strlen(kr->pd->upn));
>  -    rp += strlen(kr->pd->upn);
>  +    memcpy(&buf->data[rp], kr->pd->upn, c);
>  +    rp += c;
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = (uint32_t) strlen(kr->ccname);
>  +    c = (uint32_t) strlen(kr->ccname);
>  +    memcpy(&buf->data[rp], &c, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    memcpy(&buf->data[rp], kr->ccname, strlen(kr->ccname));
>  +    memcpy(&buf->data[rp], kr->ccname, c);
>       rp += strlen(kr->ccname);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = (uint32_t) strlen(keytab);
>  +    c = (uint32_t) strlen(keytab);
>  +    memcpy(&buf->data[rp], &c, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>  -    memcpy(&buf->data[rp], keytab, strlen(keytab));
>  +    memcpy(&buf->data[rp], keytab, c);
>       rp += strlen(keytab);
>   
>  -    ((uint32_t *)(&buf->data[rp]))[0] = kr->pd->authtok_size;
>  +    memcpy(&buf->data[rp], &kr->pd->authtok_size, sizeof(uint32_t));
>       rp += sizeof(uint32_t);
>   
>       memcpy(&buf->data[rp], kr->pd->authtok, kr->pd->authtok_size);
>       rp += kr->pd->authtok_size;
>   
>       if (kr->pd->cmd == SSS_PAM_CHAUTHTOK) {
>  -        ((uint32_t *)(&buf->data[rp]))[0] = kr->pd->newauthtok_size;
>  +        memcpy(&buf->data[rp], &kr->pd->newauthtok_size,
> sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>   
>           memcpy(&buf->data[rp], kr->pd->newauthtok,
> kr->pd->newauthtok_size);
>  diff --git a/server/providers/krb5/krb5_child.c b/server/providers/krb5/krb5_child.c
>  index 08e16b4..f4de846 100644
>  --- a/server/providers/krb5/krb5_child.c
>  +++ b/server/providers/krb5/krb5_child.c
>  @@ -260,6 +260,7 @@ static errno_t pack_response_packet(struct response *resp, int
> status, int type,
>   {
>       int len;
>       int p=0;
>  +    int32_t c;
>   
>       len = strlen(data)+1;
>       if ((3*sizeof(int32_t) + len +1) > resp->max_size) {
>  @@ -267,13 +268,16 @@ static errno_t pack_response_packet(struct response *resp, int
> status, int type,
>           return ENOMEM;
>       }
>   
>  -    ((int32_t *)(&resp->buf[p]))[0] = status;
>  +    c = status;
>  +    memcpy(&resp->buf[p], &c, sizeof(int32_t));
>       p += sizeof(int32_t);
>   
>  -    ((int32_t *)(&resp->buf[p]))[0] = type;
>  +    c = type;
>  +    memcpy(&resp->buf[p], &c, sizeof(int32_t));
>       p += sizeof(int32_t);
>   
>  -    ((int32_t *)(&resp->buf[p]))[0] = len;
>  +    c = len;
>  +    memcpy(&resp->buf[p], &c, sizeof(int32_t));
>       p += sizeof(int32_t);
>   
>       memcpy(&resp->buf[p], data, len);
>  @@ -694,28 +698,27 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct
> pam_data *pd,
>       uint32_t len;
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    pd->cmd = *((uint32_t *)(buf + p));
>  +    memcpy(&pd->cmd, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    pd->pw_uid = *((uint32_t *)(buf + p));
>  +    memcpy(&pd->pw_uid, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    pd->gr_gid = *((uint32_t *)(buf + p));
>  +    memcpy(&pd->gr_gid, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    *validate = *((uint32_t *)(buf + p));
>  +    memcpy(validate, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = *((uint32_t *)(buf + p));
>  -    *offline = len;
>  +    memcpy(offline, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = *((uint32_t *)(buf + p));
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + len ) > size) return EINVAL;
>  @@ -724,7 +727,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct
> pam_data *pd,
>       p += len;
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = *((uint32_t *)(buf + p));
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + len ) > size) return EINVAL;
>  @@ -733,7 +736,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct
> pam_data *pd,
>       p += len;
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = *((uint32_t *)(buf + p));
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + len ) > size) return EINVAL;
>  @@ -742,7 +745,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct
> pam_data *pd,
>       p += len;
>   
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = *((uint32_t *)(buf + p));
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + len) > size) return EINVAL;
>  @@ -753,7 +756,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct
> pam_data *pd,
>   
>       if (pd->cmd == SSS_PAM_CHAUTHTOK) {
>           if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -        len = *((uint32_t *)(buf + p));
>  +        memcpy(&len, buf + p, sizeof(uint32_t));
>           p += sizeof(uint32_t);
>   
>           if ((p + len) > size) return EINVAL;
>  diff --git a/server/providers/ldap/ldap_child.c b/server/providers/ldap/ldap_child.c
>  index a17bcad..448a9cc 100644
>  --- a/server/providers/ldap/ldap_child.c
>  +++ b/server/providers/ldap/ldap_child.c
>  @@ -54,7 +54,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
>           DEBUG(1, ("Error: buffer too big!\n"));
>           return EINVAL;
>       }
>  -    len = ((uint32_t *)(buf + p))[0];
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       DEBUG(7, ("realm_str size: %d\n", len));
>  @@ -68,7 +68,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
>   
>       /* princ_str size and length */
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = ((uint32_t *)(buf + p))[0];
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       DEBUG(7, ("princ_str size: %d\n", len));
>  @@ -82,7 +82,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
>   
>       /* keytab_name size and length */
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = ((uint32_t *)(buf + p))[0];
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       DEBUG(7, ("keytab_name size: %d\n", len));
>  @@ -101,16 +101,19 @@ static int pack_buffer(struct response *r, int result, const char
> *msg)
>   {
>       int len;
>       int p = 0;
>  +    uint32_t c;
>   
>       len = strlen(msg);
>       r->size = 2 * sizeof(uint32_t) + len;
>   
>       /* result */
>  -    ((uint32_t *)(&r->buf[p]))[0] = result;
>  +    c = result;
>  +    memcpy(&r->buf[p], &c, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       /* message size */
>  -    ((uint32_t *)(&r->buf[p]))[0] = len;
>  +    c = len;
>  +    memcpy(&r->buf[p], &c, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       /* message itself */
>  diff --git a/server/providers/ldap/sdap_child_helpers.c
> b/server/providers/ldap/sdap_child_helpers.c
>  index 862dacf..7f743d7 100644
>  --- a/server/providers/ldap/sdap_child_helpers.c
>  +++ b/server/providers/ldap/sdap_child_helpers.c
>  @@ -168,36 +168,39 @@ static errno_t create_tgt_req_send_buffer(TALLOC_CTX *mem_ctx,
>       /* realm */
>       if (realm_str) {
>           len = strlen(realm_str);
>  -        ((uint32_t *)(&buf->data[rp]))[0] = len;
>  +        memcpy(&buf->data[rp], &len, sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>           memcpy(&buf->data[rp], realm_str, len);
>           rp += len;
>       } else {
>  -        ((uint32_t *)(&buf->data[rp]))[0] = 0;
>  +        len = 0;
>  +        memcpy(&buf->data[rp], &len, sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>       }
>   
>       /* principal */
>       if (princ_str) {
>           len = strlen(princ_str);
>  -        ((uint32_t *)(&buf->data[rp]))[0] = len;
>  +        memcpy(&buf->data[rp], &len, sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>           memcpy(&buf->data[rp], princ_str, len);
>           rp += len;
>       } else {
>  -        ((uint32_t *)(&buf->data[rp]))[0] = 0;
>  +        len = 0;
>  +        memcpy(&buf->data[rp], &len, sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>       }
>   
>       /* keytab */
>       if (keytab_name) {
>           len = strlen(keytab_name);
>  -        ((uint32_t *)(&buf->data[rp]))[0] = len;
>  +        memcpy(&buf->data[rp], &len, sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>           memcpy(&buf->data[rp], keytab_name, len);
>           rp += len;
>       } else {
>  -        ((uint32_t *)(&buf->data[rp]))[0] = 0;
>  +        len = 0;
>  +        memcpy(&buf->data[rp], &len, sizeof(uint32_t));
>           rp += sizeof(uint32_t);
>       }
>   
>  @@ -214,14 +217,14 @@ static int parse_child_response(TALLOC_CTX *mem_ctx,
>       uint32_t res;
>       char *ccn;
>   
>  -    /* operatoin result code */
>  +    /* operation result code */
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    res = *((uint32_t *)(buf + p));
>  +    memcpy(&res, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       /* ccache name size */
>       if ((p + sizeof(uint32_t)) > size) return EINVAL;
>  -    len = *((uint32_t *)(buf + p));
>  +    memcpy(&len, buf + p, sizeof(uint32_t));
>       p += sizeof(uint32_t);
>   
>       if ((p + len ) > size) return EINVAL;
>  -- 
>  1.6.6
>   

>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/10/2010 08:09 AM, Sumit Bose wrote:
>  On Wed, Feb 10, 2010 at 01:21:04PM +0100, Jakub Hrozek wrote:
>  On 02/10/2010 11:55 AM, Sumit Bose wrote:
> >>> This patch works on i686, too. Please remove the FIXME. The assignment
> >>> was introduced in commit 2b4d76971e4ad803b42051fddb3add9fe2215dc2 with
> >>> no special reason.
> >>>
> >>> bye,
> >>> Sumit
>  
>  attached
>  
> > ACK
>  
> > bye,
> > Sumit
>   
Pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktyuSMACgkQeiVVYja6o6OOigCfcgiiK8DGaZxRtnKFZ0Tesnhf
> S6cAnR/1Xc27APcpcaNKjYp2a64+Gsx7
> =8EWg
> -----END PGP SIGNATURE-----