https://fedorahosted.org/sssd/ticket/1359
This patch mostly silences some alignment related warnings reported by
clang, but also fixes some real alignment issues.
But not all warnings are suppressed in this patch. These cases still
generate warnings:
1)=====================================================
src/util/refcount.c:63:25: warning: cast from 'char *' to 'int *'
increases required alignment from 1 to 4 [-Wcast-align]
wrapper->refcount = (int *)((char *)wrapper->ptr + refcount_offset);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/util/refcount.c:82:25: warning: cast from 'char *' to 'int *'
increases required alignment from 1 to 4 [-Wcast-align]
wrapper->refcount = (int *)((char *)wrapper->ptr + refcount_offset);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment: This is not a problem, the data we access here is an integer
that is part of a structure, so there should be no alignment issues (C
compiler would not create a structure with inaccessible structure
member). I think suppressing this warning with some additional casts
would only confuse us later and make the code more obfuscated than it is.
2)=====================================================
src/providers/ldap/sdap_async_sudo_hostinfo.c:233:24: warning: cast from
'struct sockaddr *' to 'struct sockaddr_in *' increases
required alignment from 2 to 4 [-Wcast-align]
ip4_addr = (struct sockaddr_in*)(iface->ifa_addr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/providers/ldap/sdap_async_sudo_hostinfo.c:267:27: warning: cast from
'struct sockaddr *' to 'struct sockaddr_in6 *' increases
required alignment from 2 to 4 [-Wcast-align]
ip6_network = (struct sockaddr_in6*)(iface->ifa_netmask);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...and other sockaddr related casts...
Comment: We only have to make sure that sockaddr is not allocated on
stack here. But we use the getifaddrs() function for this, so it should
be OK.
3)=====================================================
src/sss_client/nss_services.c:137:29: warning: cast from 'char *' to
'char **' increases required alignment from 1 to 8
[-Wcast-align]
sr->result->s_aliases = (char **) &(sr->buffer[i+pad]);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment: Well, there is nothing we can do about this warning. We just
make sure that the accessed data is aligned to sizeof(char **) using the
pad variable (in the original code, we aligned it to 32 bits, but that
was incorrect).
4)=====================================================
src/sss_client/nss_mc_group.c:67:22: warning: cast from 'char *' to
'char **' increases required alignment from 1 to 8 [-Wcast-align]
result->gr_mem = (char **)buffer;
^~~~~~~~~~~~~~~
src/monitor/monitor.c:1588:16: warning: cast from 'char *' to 'struct
inotify_event *' increases required alignment from 1 to 4
[-Wcast-align]
in_event = (struct inotify_event *)buf;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/nss/nsssrv_mmap_cache.c:281:22: warning: cast from 'char
*' to 'rel_ptr_t *' (aka 'unsigned int *') increases required
alignment from 1 to 4 [-Wcast-align]
name_ptr = *((rel_ptr_t *)rec->data);
^~~~~~~~~~~~~~~~~~~~~~
Comment: Here we access the beginnings of allocated memory spaces. So it
should be OK too. Adding some additional casts just to suppress the
warnings is here IMO inappropriate (it is then not so clear what data
type we use).
5)======================================================
src/responder/common/responder_packet.c:79:21: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->len = &((uint32_t *)packet->buffer)[0];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:80:21: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->cmd = &((uint32_t *)packet->buffer)[1];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:81:24: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->status = &((uint32_t *)packet->buffer)[2];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:82:26: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->reserved = &((uint32_t *)packet->buffer)[3];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:83:33: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->body = (uint8_t *)&((uint32_t *)packet->buffer)[4];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:128:29: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->len = &((uint32_t *)packet->buffer)[0];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:129:29: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->cmd = &((uint32_t *)packet->buffer)[1];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:130:32: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->status = &((uint32_t *)packet->buffer)[2];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:131:34: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->reserved = &((uint32_t *)packet->buffer)[3];
^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:132:41: warning: cast from
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
(aka 'unsigned int *') increases required alignment from 1 to 4
[-Wcast-align]
packet->body = (uint8_t *)&((uint32_t *)packet->buffer)[4];
^~~~~~~~~~~~~~~~~~~~~~~~~~
Comment: Same as number 4, but we also use addresses that are multiples
of the sizeof(data_type_used) (relative to the beginning of the buffer).
Which is OK.
================================================================
The rest of the warnings are suppressed by this patch.
I also have one question. Does talloc guarantee that the allocated block
of memory (without the talloc related metadata) is aligned to the
sizeof(biggest_primitive_C_data_type)? I know malloc does this, but I am
not sure about talloc. I saw some places in the code, where we rely on
this feature, so my guess is that talloc does it. Can someone confirm
it? The above mentioned 3, 4 and 5 also rely on proper aligned memory block.
The patch is in attachment.
Thanks
Michal
>From 0e2ce41e79ce82ddc06dab7199bfc5acbbf1c2f7 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Mon, 8 Oct 2012 16:40:03 +0200
Subject: [PATCH] Alignment issues reported by clang.
This patch mostly silences some warnings, but also
fixes some real alignment issues.
https://fedorahosted.org/sssd/ticket/1359
---
src/providers/krb5/krb5_child_handler.c | 14 ++++++-------
src/responder/autofs/autofssrv_cmd.c | 11 ++++++++--
src/responder/common/responder_cmd.c | 19 ++++++++++++-----
src/responder/nss/nsssrv_cmd.c | 34 ++++++++++++++++++++----------
src/responder/nss/nsssrv_netgroup.c | 18 +++++++++++-----
src/responder/nss/nsssrv_services.c | 8 ++++---
src/sss_client/autofs/sss_autofs.c | 6 +++++-
src/sss_client/common.c | 2 +-
src/sss_client/nss_group.c | 37 +++++++++++++++++++++++----------
src/sss_client/nss_netgroup.c | 12 +++++++++--
src/sss_client/nss_passwd.c | 22 +++++++++++++++-----
src/sss_client/nss_services.c | 28 +++++++++++++++++--------
src/sss_client/pam_sss.c | 11 +++++-----
13 files changed, 154 insertions(+), 68 deletions(-)
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 768d8c7..2fd1b7f 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -435,8 +435,8 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len,
int32_t msg_len;
int64_t time_data;
struct tgt_times tgtt;
- uint32_t *expiration;
- uint32_t *msg_subtype;
+ uint32_t expiration;
+ uint32_t msg_subtype;
struct krb5_child_response *res;
if ((size_t) len < sizeof(int32_t)) {
@@ -503,12 +503,12 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len,
}
if (msg_type == SSS_PAM_USER_INFO) {
- msg_subtype = (uint32_t *)&buf[p];
- if (*msg_subtype == SSS_PAM_USER_INFO_EXPIRE_WARN)
- {
- expiration = (uint32_t *)&buf[p+sizeof(uint32_t)];
+ SAFEALIGN_COPY_UINT32(&msg_subtype, buf + p, NULL);
+ if (msg_subtype == SSS_PAM_USER_INFO_EXPIRE_WARN) {
+ SAFEALIGN_COPY_UINT32(&expiration,
+ buf+p+sizeof(uint32_t), NULL);
if (pwd_exp_warning > 0 &&
- difftime(pwd_exp_warning, *expiration) < 0.0) {
+ difftime(pwd_exp_warning, expiration) < 0.0) {
skip = true;
}
}
diff --git a/src/responder/autofs/autofssrv_cmd.c b/src/responder/autofs/autofssrv_cmd.c
index 3af4a84..5c2baed 100644
--- a/src/responder/autofs/autofssrv_cmd.c
+++ b/src/responder/autofs/autofssrv_cmd.c
@@ -240,6 +240,8 @@ static void sss_autofs_cmd_setautomntent_done(struct tevent_req *req)
struct sss_packet *packet;
uint8_t *body;
size_t blen;
+ size_t num_bytes;
+ uint32_t number_32bit;
DEBUG(SSSDBG_TRACE_INTERNAL, ("setautomntent done\n"));
@@ -271,8 +273,13 @@ static void sss_autofs_cmd_setautomntent_done(struct tevent_req *req)
}
sss_packet_get_body(packet, &body, &blen);
- ((uint32_t *)body)[0] = 1; /* Got some results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+
+ num_bytes = 0;
+ number_32bit = 1; /* Got some results */
+ SAFEALIGN_COPY_UINT32(body, &number_32bit, &num_bytes);
+
+ number_32bit = 0; /* reserved */
+ SAFEALIGN_COPY_UINT32(body + num_bytes, &number_32bit, NULL);
}
sss_cmd_done(cmdctx->cctx, NULL);
diff --git a/src/responder/common/responder_cmd.c b/src/responder/common/responder_cmd.c
index 16f38ea..5c4d7c2 100644
--- a/src/responder/common/responder_cmd.c
+++ b/src/responder/common/responder_cmd.c
@@ -45,14 +45,17 @@ int sss_cmd_empty_packet(struct sss_packet *packet)
{
uint8_t *body;
size_t blen;
+ size_t num_bytes;
+ uint32_t zero_32bit = 0;
int ret;
ret = sss_packet_grow(packet, 2*sizeof(uint32_t));
if (ret != EOK) return ret;
sss_packet_get_body(packet, &body, &blen);
- ((uint32_t *)body)[0] = 0; /* num results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+ num_bytes = 0;
+ SAFEALIGN_COPY_UINT32(body, &zero_32bit, &num_bytes); /* 0 results */
+ SAFEALIGN_COPY_UINT32(body + num_bytes, &zero_32bit, NULL); /* reserved */
return EOK;
}
@@ -97,6 +100,7 @@ int sss_cmd_get_version(struct cli_ctx *cctx)
size_t blen;
int ret;
uint32_t client_version;
+ uint32_t ret_version;
int i;
static struct cli_protocol_version *cli_protocol_version = NULL;
@@ -133,9 +137,14 @@ int sss_cmd_get_version(struct cli_ctx *cctx)
return ret;
}
sss_packet_get_body(cctx->creq->out, &body, &blen);
- ((uint32_t *)body)[0] = cctx->cli_protocol_version!=NULL ?
- cctx->cli_protocol_version->version : 0;
- DEBUG(5, ("Offered version [%d].\n", ((uint32_t *)body)[0]));
+
+ if (cctx->cli_protocol_version != NULL) {
+ ret_version = cctx->cli_protocol_version->version;
+ } else {
+ ret_version = 0;
+ }
+ SAFEALIGN_COPY_UINT32(body, &ret_version, NULL);
+ DEBUG(5, ("Offered version [%d].\n", ret_version));
sss_cmd_done(cctx, NULL);
return EOK;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 370c3d2..c129555 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -239,6 +239,7 @@ static int fill_pwent(struct sss_packet *packet,
struct sized_string fullname;
uint32_t uid;
uint32_t gid;
+ uint32_t zero_32bit = 0;
size_t rsize, rp, blen;
size_t dom_len = 0;
int delim = 1;
@@ -398,8 +399,10 @@ done:
if (!packet_initialized) return ENOENT;
sss_packet_get_body(packet, &body, &blen);
- ((uint32_t *)body)[0] = num; /* num results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+
+ rp = 0;
+ SAFEALIGN_COPY_UINT32(body, &num, &rp); /* num results */
+ SAFEALIGN_COPY_UINT32(body + rp, &zero_32bit, NULL); /* reserved */
return EOK;
}
@@ -1121,7 +1124,8 @@ static int nss_cmd_getpwuid(struct cli_ctx *cctx)
ret = EINVAL;
goto done;
}
- cmdctx->id = *((uint32_t *)body);
+
+ SAFEALIGN_COPY_UINT32(&cmdctx->id, body, NULL);
ret = sss_ncache_check_uid(nctx->ncache, nctx->neg_timeout, cmdctx->id);
if (ret == EEXIST) {
@@ -1571,7 +1575,7 @@ static int nss_cmd_getpwent_immediate(struct nss_cmd_ctx *cmdctx)
if (blen != sizeof(uint32_t)) {
return EINVAL;
}
- num = *((uint32_t *)body);
+ SAFEALIGN_COPY_UINT32(&num, body, NULL);
/* create response packet */
ret = sss_packet_new(cctx->creq, 0,
@@ -1843,6 +1847,7 @@ static int fill_grent(struct sss_packet *packet,
uint8_t *body;
size_t blen;
uint32_t gid;
+ uint32_t zero_32bit = 0;
const char *tmpstr;
const char *orig_name;
struct sized_string name;
@@ -1850,6 +1855,7 @@ static int fill_grent(struct sss_packet *packet,
struct sized_string fullname;
size_t delim;
size_t dom_len;
+ size_t rp;
int i = 0;
int ret, num, memnum;
size_t rzero, rsize;
@@ -2045,8 +2051,9 @@ done:
return ENOENT;
}
- ((uint32_t *)body)[0] = num; /* num results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+ rp = 0;
+ SAFEALIGN_COPY_UINT32(body, &num, &rp); /* num results */
+ SAFEALIGN_COPY_UINT32(body + rp, &zero_32bit, NULL); /* reserved */
return EOK;
}
@@ -2587,7 +2594,7 @@ static int nss_cmd_getgrgid(struct cli_ctx *cctx)
ret = EINVAL;
goto done;
}
- cmdctx->id = *((uint32_t *)body);
+ SAFEALIGN_COPY_UINT32(&cmdctx->id, body, NULL);
ret = sss_ncache_check_gid(nctx->ncache, nctx->neg_timeout, cmdctx->id);
if (ret == EEXIST) {
@@ -3038,7 +3045,7 @@ static int nss_cmd_getgrent_immediate(struct nss_cmd_ctx *cmdctx)
if (blen != sizeof(uint32_t)) {
return EINVAL;
}
- num = *((uint32_t *)body);
+ SAFEALIGN_COPY_UINT32(&num, body, NULL);
/* create response packet */
ret = sss_packet_new(cctx->creq, 0,
@@ -3161,6 +3168,8 @@ static int fill_initgr(struct sss_packet *packet, struct ldb_result *res)
{
uint8_t *body;
size_t blen;
+ size_t rp;
+ uint32_t zero_32bit = 0;
gid_t gid;
int ret, i, num, bindex;
int skipped = 0;
@@ -3193,12 +3202,15 @@ static int fill_initgr(struct sss_packet *packet, struct ldb_result *res)
return EFAULT;
}
}
- ((uint32_t *)body)[2 + bindex] = gid;
+ SAFEALIGN_COPY_UINT32(body + (2 + bindex) * sizeof(uint32_t),
+ &gid, NULL);
bindex++;
}
- ((uint32_t *)body)[0] = num-skipped; /* num results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+ rp = 0;
+ num -= skipped;
+ SAFEALIGN_COPY_UINT32(body, &num, &rp); /* num results */
+ SAFEALIGN_COPY_UINT32(body + rp, &zero_32bit, NULL); /* reserved */
return EOK;
}
diff --git a/src/responder/nss/nsssrv_netgroup.c b/src/responder/nss/nsssrv_netgroup.c
index ae993fa..4e7c0bc 100644
--- a/src/responder/nss/nsssrv_netgroup.c
+++ b/src/responder/nss/nsssrv_netgroup.c
@@ -637,7 +637,9 @@ static void nss_cmd_setnetgrent_done(struct tevent_req *req)
errno_t ret;
struct sss_packet *packet;
uint8_t *body;
+ size_t rp;
size_t blen;
+ uint32_t number_32bit;
struct nss_cmd_ctx *cmdctx =
tevent_req_callback_data(req, struct nss_cmd_ctx);
@@ -667,8 +669,13 @@ static void nss_cmd_setnetgrent_done(struct tevent_req *req)
}
sss_packet_get_body(packet, &body, &blen);
- ((uint32_t *)body)[0] = 1; /* Got some results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+
+ rp = 0;
+ number_32bit = 1; /* Got some results */
+ SAFEALIGN_COPY_UINT32(body, &number_32bit, &rp);
+
+ number_32bit = 0; /* reserved */
+ SAFEALIGN_COPY_UINT32(body + rp, &number_32bit, NULL);
}
sss_cmd_done(cmdctx->cctx, NULL);
@@ -829,7 +836,7 @@ static errno_t nss_cmd_getnetgrent_process(struct nss_cmd_ctx *cmdctx,
if (blen != sizeof(uint32_t)) {
return EINVAL;
}
- num = *((uint32_t *)body);
+ SAFEALIGN_COPY_UINT32(&num, body, NULL);
/* create response packet */
ret = sss_packet_new(client->creq, 0,
@@ -872,6 +879,7 @@ static errno_t nss_cmd_retnetgrent(struct cli_ctx *client,
errno_t ret;
struct sss_packet *packet = client->creq->out;
int num, start;
+ uint32_t zero_32bit = 0;
/* first 2 fields (len and reserved), filled up later */
rp = 2*sizeof(uint32_t);
@@ -968,8 +976,8 @@ static errno_t nss_cmd_retnetgrent(struct cli_ctx *client,
}
sss_packet_get_body(packet, &body, &blen);
- ((uint32_t *)body)[0] = num; /* num results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+ SAFEALIGN_COPY_UINT32(body, &num, &rp); /* num results */
+ SAFEALIGN_COPY_UINT32(body + rp, &zero_32bit, NULL); /* reserved */
return EOK;
}
diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
index d79323c..c8b7f7c 100644
--- a/src/responder/nss/nsssrv_services.c
+++ b/src/responder/nss/nsssrv_services.c
@@ -600,6 +600,7 @@ fill_service(struct sss_packet *packet,
unsigned int num = 0;
unsigned int i, j;
uint32_t num_aliases, written_aliases;
+ uint32_t zero_32bit = 0;
struct ldb_message *msg;
struct ldb_message_element *el;
TALLOC_CTX *tmp_ctx = NULL;
@@ -768,8 +769,9 @@ done:
return ENOENT;
}
- ((uint32_t *)body)[0] = num; /* num results */
- ((uint32_t *)body)[1] = 0; /* reserved */
+ aptr = 0;
+ SAFEALIGN_COPY_UINT32(body, &num, &aptr); /* num results */
+ SAFEALIGN_COPY_UINT32(body + aptr, &zero_32bit, NULL); /* reserved */
return ret;
}
@@ -1738,7 +1740,7 @@ nss_cmd_getservent_immediate(struct nss_cmd_ctx *cmdctx)
if (blen != sizeof(uint32_t)) {
return EINVAL;
}
- num = *((uint32_t *)body);
+ SAFEALIGN_COPY_UINT32(body, &num, NULL);
/* create response packet */
ret = sss_packet_new(cctx->creq, 0,
diff --git a/src/sss_client/autofs/sss_autofs.c b/src/sss_client/autofs/sss_autofs.c
index e87ef4f..a690718 100644
--- a/src/sss_client/autofs/sss_autofs.c
+++ b/src/sss_client/autofs/sss_autofs.c
@@ -64,6 +64,7 @@ _sss_setautomntent(const char *mapname, void **context)
struct sss_cli_req_data rd;
uint8_t *repbuf = NULL;
size_t replen;
+ uint32_t num_results = 0;
if (!mapname) return EINVAL;
@@ -96,8 +97,11 @@ _sss_setautomntent(const char *mapname, void **context)
goto out;
}
+ /* Get number of results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(name);
free(repbuf);
ret = ENOENT;
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 5b17515..55246c1 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -417,7 +417,7 @@ static bool sss_cli_check_version(const char *socket_name)
return false;
}
- obtained_version = ((uint32_t *)repbuf)[0];
+ SAFEALIGN_COPY_UINT32(&obtained_version, repbuf, NULL);
free(repbuf);
return (obtained_version == expected_version);
diff --git a/src/sss_client/nss_group.c b/src/sss_client/nss_group.c
index 99b7ad9..9d5b8f5 100644
--- a/src/sss_client/nss_group.c
+++ b/src/sss_client/nss_group.c
@@ -234,9 +234,9 @@ static int sss_nss_getgr_readrep(struct sss_nss_gr_rep *pr,
NULL);
if (ret != EOK) return ret;
- /* Make sure pr->buffer[i+pad] is 32 bit aligned */
+ /* Make sure pr->buffer[i+pad] is aligned to sizeof (char **) */
pad = 0;
- while((i + pad) % 4) {
+ while((i + pad) % sizeof(char **)) {
pad++;
}
@@ -283,7 +283,7 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
uint8_t *repbuf;
size_t replen;
enum nss_status nret;
- uint32_t *rbuf;
+ size_t buf_index = 0;
uint32_t num_ret;
long int l, max_ret;
@@ -299,7 +299,7 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
}
/* no results if not found */
- num_ret = ((uint32_t *)repbuf)[0];
+ SAFEALIGN_COPY_UINT32(&num_ret, repbuf, NULL);
if (num_ret == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
@@ -329,9 +329,12 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
*size = newsize;
}
- rbuf = &((uint32_t *)repbuf)[2];
+ /* Skip number of results and reserved padding. */
+ buf_index = 2 * sizeof(uint32_t);
+
for (l = 0; l < max_ret; l++) {
- (*groups)[*start] = rbuf[l];
+ SAFEALIGN_COPY_UINT32(&(*groups)[*start], repbuf + buf_index,
+ &buf_index);
*start += 1;
}
@@ -351,6 +354,7 @@ enum nss_status _nss_sss_getgrnam_r(const char *name, struct group *result,
struct sss_nss_gr_rep grrep;
uint8_t *repbuf;
size_t replen, len, name_len;
+ uint32_t num_results;
enum nss_status nret;
int ret;
@@ -403,15 +407,18 @@ enum nss_status _nss_sss_getgrnam_r(const char *name, struct group *result,
grrep.buffer = buffer;
grrep.buflen = buflen;
+ /* Get number of results from repbuf. */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
}
/* only 1 result is accepted for this function */
- if (((uint32_t *)repbuf)[0] != 1) {
+ if (num_results != 1) {
*errnop = EBADMSG;
free(repbuf);
nret = NSS_STATUS_TRYAGAIN;
@@ -445,6 +452,7 @@ enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
struct sss_nss_gr_rep grrep;
uint8_t *repbuf;
size_t replen, len;
+ uint32_t num_results;
enum nss_status nret;
uint32_t group_gid;
int ret;
@@ -490,15 +498,18 @@ enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
grrep.buffer = buffer;
grrep.buflen = buflen;
+ /* Get number of results from repbuf. */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
}
/* only 1 result is accepted for this function */
- if (((uint32_t *)repbuf)[0] != 1) {
+ if (num_results != 1) {
*errnop = EBADMSG;
free(repbuf);
nret = NSS_STATUS_TRYAGAIN;
@@ -553,6 +564,7 @@ static enum nss_status internal_getgrent_r(struct group *result,
struct sss_nss_gr_rep grrep;
uint8_t *repbuf;
size_t replen;
+ uint32_t num_results;
enum nss_status nret;
uint32_t num_entries;
int ret;
@@ -599,8 +611,11 @@ static enum nss_status internal_getgrent_r(struct group *result,
return nret;
}
+ /* Get number of results from repbuf. */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if ((((uint32_t *)repbuf)[0] == 0) || (replen - 8 == 0)) {
+ if ((num_results == 0) || (replen - 8 == 0)) {
free(repbuf);
return NSS_STATUS_NOTFOUND;
}
diff --git a/src/sss_client/nss_netgroup.c b/src/sss_client/nss_netgroup.c
index f72d547..8594fc4 100644
--- a/src/sss_client/nss_netgroup.c
+++ b/src/sss_client/nss_netgroup.c
@@ -160,6 +160,7 @@ enum nss_status _nss_sss_setnetgrent(const char *netgroup,
{
uint8_t *repbuf = NULL;
size_t replen;
+ uint32_t num_results;
enum nss_status nret;
struct sss_cli_req_data rd;
int errnop;
@@ -198,8 +199,11 @@ enum nss_status _nss_sss_setnetgrent(const char *netgroup,
goto out;
}
+ /* Get number of results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if ((((uint32_t *)repbuf)[0] == 0) || (replen < NETGR_METADATA_COUNT)) {
+ if ((num_results == 0) || (replen < NETGR_METADATA_COUNT)) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
@@ -221,6 +225,7 @@ static enum nss_status internal_getnetgrent_r(struct __netgrent *result,
struct sss_nss_netgr_rep netgrrep;
uint8_t *repbuf;
size_t replen;
+ uint32_t num_results;
enum nss_status nret;
uint32_t num_entries;
int ret;
@@ -266,8 +271,11 @@ static enum nss_status internal_getnetgrent_r(struct __netgrent *result,
return nret;
}
+ /* Get number of results from repbuf. */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if ((((uint32_t *)repbuf)[0] == 0) || (replen <= NETGR_METADATA_COUNT)) {
+ if ((num_results == 0) || (replen <= NETGR_METADATA_COUNT)) {
free(repbuf);
return NSS_STATUS_RETURN;
}
diff --git a/src/sss_client/nss_passwd.c b/src/sss_client/nss_passwd.c
index e2f488b..049f7b1 100644
--- a/src/sss_client/nss_passwd.c
+++ b/src/sss_client/nss_passwd.c
@@ -140,6 +140,7 @@ enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
struct sss_nss_pw_rep pwrep;
uint8_t *repbuf;
size_t replen, len, name_len;
+ uint32_t num_results;
enum nss_status nret;
int ret;
@@ -188,15 +189,18 @@ enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
pwrep.buffer = buffer;
pwrep.buflen = buflen;
+ /* Get number of results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
}
/* only 1 result is accepted for this function */
- if (((uint32_t *)repbuf)[0] != 1) {
+ if (num_results != 1) {
*errnop = EBADMSG;
free(repbuf);
nret = NSS_STATUS_TRYAGAIN;
@@ -226,6 +230,7 @@ enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
struct sss_nss_pw_rep pwrep;
uint8_t *repbuf;
size_t replen, len;
+ uint32_t num_results;
enum nss_status nret;
uint32_t user_uid;
int ret;
@@ -267,15 +272,18 @@ enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
pwrep.buffer = buffer;
pwrep.buflen = buflen;
+ /* Get number of results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
}
/* only 1 result is accepted for this function */
- if (((uint32_t *)repbuf)[0] != 1) {
+ if (num_results != 1) {
*errnop = EBADMSG;
free(repbuf);
nret = NSS_STATUS_TRYAGAIN;
@@ -326,6 +334,7 @@ static enum nss_status internal_getpwent_r(struct passwd *result,
struct sss_nss_pw_rep pwrep;
uint8_t *repbuf;
size_t replen;
+ uint32_t num_results;
enum nss_status nret;
uint32_t num_entries;
int ret;
@@ -370,8 +379,11 @@ static enum nss_status internal_getpwent_r(struct passwd *result,
return nret;
}
+ /* Get number of results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if ((((uint32_t *)repbuf)[0] == 0) || (replen - 8 == 0)) {
+ if ((num_results == 0) || (replen - 8 == 0)) {
free(repbuf);
return NSS_STATUS_NOTFOUND;
}
diff --git a/src/sss_client/nss_services.c b/src/sss_client/nss_services.c
index 5f98d8d..642c690 100644
--- a/src/sss_client/nss_services.c
+++ b/src/sss_client/nss_services.c
@@ -127,15 +127,14 @@ sss_nss_getsvc_readrep(struct sss_nss_svc_rep *sr,
NULL);
if (ret != EOK) return ret;
- /* Make sure sr->buffer[i+pad] is 32-bit aligned */
+ /* Make sure sr->buffer[i+pad] is aligned to sizeof(char **) bytes */
pad = 0;
- while((i + pad) % 4) {
+ while((i + pad) % (sizeof(char **))) {
pad++;
}
/* Copy in the aliases */
sr->result->s_aliases = (char **) &(sr->buffer[i+pad]);
-
ptaliases = (sizeof(char *) * (num_aliases + 1)) + pad;
if (ptaliases > dlen) {
return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
@@ -175,6 +174,7 @@ _nss_sss_getservbyname_r(const char *name,
uint8_t *repbuf;
uint8_t *data;
size_t replen, len;
+ uint32_t num_results;
enum nss_status nret;
int ret;
@@ -225,15 +225,18 @@ _nss_sss_getservbyname_r(const char *name,
svcrep.buffer = buffer;
svcrep.buflen = buflen;
+ /* Get number of results from repbuf. */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
}
/* only 1 result is accepted for this function */
- if (((uint32_t *)repbuf)[0] != 1) {
+ if (num_results != 1) {
*errnop = EBADMSG;
free(repbuf);
nret = NSS_STATUS_TRYAGAIN;
@@ -272,6 +275,7 @@ _nss_sss_getservbyport_r(int port, const char *protocol,
uint8_t *data;
size_t p = 0;
size_t replen, len;
+ uint32_t num_results;
enum nss_status nret;
int ret;
@@ -320,15 +324,18 @@ _nss_sss_getservbyport_r(int port, const char *protocol,
svcrep.buffer = buffer;
svcrep.buflen = buflen;
+ /* Get number if results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if (((uint32_t *)repbuf)[0] == 0) {
+ if (num_results == 0) {
free(repbuf);
nret = NSS_STATUS_NOTFOUND;
goto out;
}
/* only 1 result is accepted for this function */
- if (((uint32_t *)repbuf)[0] != 1) {
+ if (num_results != 1) {
*errnop = EBADMSG;
free(repbuf);
nret = NSS_STATUS_TRYAGAIN;
@@ -400,6 +407,7 @@ static enum nss_status internal_getservent_r(struct servent *result,
struct sss_nss_svc_rep pwrep;
uint8_t *repbuf;
size_t replen;
+ uint32_t num_results;
enum nss_status nret;
uint32_t num_entries;
int ret;
@@ -444,9 +452,11 @@ static enum nss_status internal_getservent_r(struct servent *result,
return nret;
}
+ /* Get number of results from repbuf */
+ SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+
/* no results if not found */
- if ((((uint32_t *)repbuf)[0] == 0)
- || (replen - SVC_METADATA_COUNT == 0)) {
+ if ((num_results == 0) || (replen - SVC_METADATA_COUNT == 0)) {
free(repbuf);
return NSS_STATUS_NOTFOUND;
}
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index efbc48b..3e6fc87 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -216,7 +216,8 @@ static int pack_message_v3(struct pam_items *pi, size_t *size,
uint8_t **buffer) {
int len;
uint8_t *buf;
- int rp;
+ size_t rp;
+ uint32_t start = SSS_START_OF_PAM_REQUEST;
uint32_t terminator = SSS_END_OF_PAM_REQUEST;
len = sizeof(uint32_t) +
@@ -243,8 +244,7 @@ static int pack_message_v3(struct pam_items *pi, size_t *size,
}
rp = 0;
- ((uint32_t *)(&buf[rp]))[0] = SSS_START_OF_PAM_REQUEST;
- rp += sizeof(uint32_t);
+ SAFEALIGN_COPY_UINT32(buf, &start, &rp);
rp += add_string_item(SSS_PAM_ITEM_USER, pi->pam_user, pi->pam_user_size,
&buf[rp]);
@@ -271,8 +271,7 @@ static int pack_message_v3(struct pam_items *pi, size_t *size,
pi->pam_newauthtok, pi->pam_newauthtok_size,
&buf[rp]);
- memcpy(&buf[rp], &terminator, sizeof(uint32_t));
- rp += sizeof(uint32_t);
+ SAFEALIGN_COPY_UINT32(buf + rp, &terminator, &rp);
if (rp != len) {
D(("error during packet creation."));
@@ -1093,7 +1092,7 @@ static int send_and_receive(pam_handle_t *pamh, struct pam_items *pi,
goto done;
}
- pam_status = ((int32_t *)repbuf)[0];
+ SAFEALIGN_COPY_UINT32(&pam_status, repbuf, NULL);
ret = eval_response(pamh, replen, repbuf, pi);
if (ret != PAM_SUCCESS) {
D(("eval_response failed."));
--
1.7.11.2
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
