Avoid using memcpy on unaligned uint16_t variables, because certain compilers will optimize this code to a direct copy instruction that will result in unaligned memory access. Thanks to Angelo Compagnucci for reporting this problem.

This commit is contained in:
Niclas Finne 2012-03-31 01:25:27 +02:00
parent ed92994784
commit 2507ba4e8d
4 changed files with 42 additions and 41 deletions

View file

@ -56,7 +56,6 @@
#endif #endif
#include <string.h> #include <string.h>
#include <stdio.h>
#include <stddef.h> #include <stddef.h>
struct announcement_data { struct announcement_data {
@ -131,32 +130,31 @@ static void
adv_packet_received(struct broadcast_conn *ibc, const rimeaddr_t *from) adv_packet_received(struct broadcast_conn *ibc, const rimeaddr_t *from)
{ {
struct announcement_msg adata; struct announcement_msg adata;
struct announcement_data data;
uint8_t *ptr;
int i; int i;
ptr = packetbuf_dataptr();
/* Copy number of announcements */ /* Copy number of announcements */
memcpy(&adata, packetbuf_dataptr(), sizeof(struct announcement_msg)); memcpy(&adata, ptr, sizeof(struct announcement_msg));
PRINTF("%d.%d: adv_packet_received from %d.%d with %d announcements\n", PRINTF("%d.%d: adv_packet_received from %d.%d with %d announcements\n",
rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1], rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1],
from->u8[0], from->u8[1], adata.num); from->u8[0], from->u8[1], adata.num);
if(adata.num / sizeof(struct announcement_data) > sizeof(struct announcement_msg)) { if(ANNOUNCEMENT_MSG_HEADERLEN + adata.num * sizeof(struct announcement_data) > packetbuf_datalen()) {
/* The number of announcements is too large - corrupt packet has /* The number of announcements is too large - corrupt packet has
been received. */ been received. */
printf("adata.num way out there: %d\n", adata.num); PRINTF("adata.num way out there: %d\n", adata.num);
return; return;
} }
for(i = 0; i < adata.num; ++i) {
struct announcement_data data;
ptr += ANNOUNCEMENT_MSG_HEADERLEN;
for(i = 0; i < adata.num; ++i) {
/* Copy announcements */ /* Copy announcements */
memcpy(&data.id, &((struct announcement_msg *)packetbuf_dataptr())->data[i].id, memcpy(&data, ptr, sizeof(struct announcement_data));
sizeof(uint16_t)); announcement_heard(from, data.id, data.value);
memcpy(&data.value, &((struct announcement_msg *)packetbuf_dataptr())->data[i].value, ptr += sizeof(struct announcement_data);
sizeof(uint16_t));
announcement_heard(from,
data.id,
data.value);
} }
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/

View file

@ -758,8 +758,7 @@ send_next_packet(struct collect_conn *tc)
static void static void
handle_ack(struct collect_conn *tc) handle_ack(struct collect_conn *tc)
{ {
struct ack_msg *msg; struct ack_msg msg;
uint16_t rtmetric;
struct collect_neighbor *n; struct collect_neighbor *n;
PRINTF("handle_ack: sender %d.%d current_parent %d.%d, id %d seqno %d\n", PRINTF("handle_ack: sender %d.%d current_parent %d.%d, id %d seqno %d\n",
@ -778,8 +777,7 @@ handle_ack(struct collect_conn *tc)
(int)(((100 * (clock_time() - tc->send_time)) / CLOCK_SECOND) % 100));*/ (int)(((100 * (clock_time() - tc->send_time)) / CLOCK_SECOND) % 100));*/
stats.ackrecv++; stats.ackrecv++;
msg = packetbuf_dataptr(); memcpy(&msg, packetbuf_dataptr(), sizeof(struct ack_msg));
memcpy(&rtmetric, &msg->rtmetric, sizeof(uint16_t));
/* It is possible that we receive an ACK for a packet that we /* It is possible that we receive an ACK for a packet that we
think we have not yet sent: if our transmission was received by think we have not yet sent: if our transmission was received by
@ -797,7 +795,7 @@ handle_ack(struct collect_conn *tc)
if(n != NULL) { if(n != NULL) {
collect_neighbor_tx(n, tc->transmissions); collect_neighbor_tx(n, tc->transmissions);
collect_neighbor_update_rtmetric(n, rtmetric); collect_neighbor_update_rtmetric(n, msg.rtmetric);
update_rtmetric(tc); update_rtmetric(tc);
} }
@ -805,8 +803,8 @@ handle_ack(struct collect_conn *tc)
rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1], rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1],
tc->current_parent.u8[0], tc->current_parent.u8[1], tc->current_parent.u8[0], tc->current_parent.u8[1],
tc->transmissions, tc->transmissions,
msg->flags, msg.flags,
rtmetric); msg.rtmetric);
/* The ack contains information about the state of the packet and /* The ack contains information about the state of the packet and
of the node that received it. We do different things depending of the node that received it. We do different things depending
@ -814,20 +812,20 @@ handle_ack(struct collect_conn *tc)
the receiving node was congested. If so, we add a maximum the receiving node was congested. If so, we add a maximum
transmission number to its routing metric, which increases the transmission number to its routing metric, which increases the
chance that another parent will be chosen. */ chance that another parent will be chosen. */
if(msg->flags & ACK_FLAGS_CONGESTED) { if(msg.flags & ACK_FLAGS_CONGESTED) {
PRINTF("ACK flag indicated parent was congested.\n"); PRINTF("ACK flag indicated parent was congested.\n");
collect_neighbor_set_congested(n); collect_neighbor_set_congested(n);
collect_neighbor_tx(n, tc->max_rexmits * 2); collect_neighbor_tx(n, tc->max_rexmits * 2);
update_rtmetric(tc); update_rtmetric(tc);
} }
if((msg->flags & ACK_FLAGS_DROPPED) == 0) { if((msg.flags & ACK_FLAGS_DROPPED) == 0) {
/* If the packet was successfully received, we send the next packet. */ /* If the packet was successfully received, we send the next packet. */
send_next_packet(tc); send_next_packet(tc);
} else { } else {
/* If the packet was lost due to its lifetime being exceeded, /* If the packet was lost due to its lifetime being exceeded,
there is not much more we can do with the packet, so we send there is not much more we can do with the packet, so we send
the next one instead. */ the next one instead. */
if((msg->flags & ACK_FLAGS_LIFETIME_EXCEEDED)) { if((msg.flags & ACK_FLAGS_LIFETIME_EXCEEDED)) {
send_next_packet(tc); send_next_packet(tc);
} else { } else {
/* If the packet was dropped, but without the node being /* If the packet was dropped, but without the node being
@ -845,7 +843,7 @@ handle_ack(struct collect_conn *tc)
/* Our neighbor's rtmetric needs to be updated, so we bump our /* Our neighbor's rtmetric needs to be updated, so we bump our
advertisements. */ advertisements. */
if(msg->flags & ACK_FLAGS_RTMETRIC_NEEDS_UPDATE) { if(msg.flags & ACK_FLAGS_RTMETRIC_NEEDS_UPDATE) {
bump_advertisement(tc); bump_advertisement(tc);
} }
set_keepalive_timer(tc); set_keepalive_timer(tc);

View file

@ -96,23 +96,22 @@ static void
adv_packet_received(struct broadcast_conn *ibc, const rimeaddr_t *from) adv_packet_received(struct broadcast_conn *ibc, const rimeaddr_t *from)
{ {
struct neighbor_discovery_conn *c = (struct neighbor_discovery_conn *)ibc; struct neighbor_discovery_conn *c = (struct neighbor_discovery_conn *)ibc;
struct adv_msg *msg = packetbuf_dataptr(); struct adv_msg msg;
uint16_t val;
memcpy(&val, &msg->val, sizeof(val)); memcpy(&msg, packetbuf_dataptr(), sizeof(struct adv_msg));
PRINTF("%d.%d: adv_packet_received from %d.%d with val %d\n", PRINTF("%d.%d: adv_packet_received from %d.%d with val %d\n",
rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1], rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1],
from->u8[0], from->u8[1], val); from->u8[0], from->u8[1], msg.val);
/* If we receive an announcement with a lower value than ours, we /* If we receive an announcement with a lower value than ours, we
cancel our own announcement. */ cancel our own announcement. */
if(val < c->val) { if(msg.val < c->val) {
/* ctimer_stop(&c->send_timer);*/ /* ctimer_stop(&c->send_timer);*/
} }
if(c->u->recv) { if(c->u->recv) {
c->u->recv(c, from, val); c->u->recv(c, from, msg.val);
} }
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/

View file

@ -125,25 +125,31 @@ static void
adv_packet_received(struct ipolite_conn *ipolite, const rimeaddr_t *from) adv_packet_received(struct ipolite_conn *ipolite, const rimeaddr_t *from)
{ {
struct announcement_msg adata; struct announcement_msg adata;
struct announcement_data data;
uint8_t *ptr;
int i; int i;
ptr = packetbuf_dataptr();
/* Copy number of announcements */ /* Copy number of announcements */
memcpy(&adata, packetbuf_dataptr(), sizeof(struct announcement_msg)); memcpy(&adata, ptr, sizeof(struct announcement_msg));
PRINTF("%d.%d: adv_packet_received from %d.%d with %d announcements\n", PRINTF("%d.%d: adv_packet_received from %d.%d with %d announcements\n",
rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1], rimeaddr_node_addr.u8[0], rimeaddr_node_addr.u8[1],
from->u8[0], from->u8[1], adata.num); from->u8[0], from->u8[1], adata.num);
for(i = 0; i < adata.num; ++i) { if(ANNOUNCEMENT_MSG_HEADERLEN + adata.num * sizeof(struct announcement_data) > packetbuf_datalen()) {
struct announcement_data data; /* The number of announcements is too large - corrupt packet has
been received. */
PRINTF("adata.num way out there: %d\n", adata.num);
return;
}
ptr += ANNOUNCEMENT_MSG_HEADERLEN;
for(i = 0; i < adata.num; ++i) {
/* Copy announcements */ /* Copy announcements */
memcpy(&data.id, &((struct announcement_msg *)packetbuf_dataptr())->data[i].id, memcpy(&data, ptr, sizeof(struct announcement_data));
sizeof(uint16_t)); announcement_heard(from, data.id, data.value);
memcpy(&data.value, &((struct announcement_msg *)packetbuf_dataptr())->data[i].value, ptr += sizeof(struct announcement_data);
sizeof(uint16_t));
announcement_heard(from,
data.id,
data.value);
} }
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/