Skip to content

Commit

Permalink
controller: fix possible unaligned accesses in DHCPv6 code
Browse files Browse the repository at this point in the history
According to RFC8415 [0], section 8 "Client/Server Message Formats":
"Options are stored serially in the "options" field, with no padding
 between the options.  Options are byte-aligned but are not aligned
 in any other way (such as on 2-byte or 4-byte boundaries)."

Fix possible unaligned accesses in DHCPv6 code reading/writing
getting rid of dhcp_opt6_header structure that trigger undefined
behaviour issues reported by ubsan sanitizer and relying on
dhcpv6_opt_header that is defined as packed.

[0] https://www.rfc-editor.org/rfc/rfc8415.html

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 8298eac)
  • Loading branch information
LorenzoBianconi authored and numansiddique committed May 9, 2023
1 parent 8b7d92c commit 57c788a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 14 deletions.
6 changes: 3 additions & 3 deletions controller/pinctrl.c
Expand Up @@ -2368,19 +2368,19 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,
struct ofpbuf *out_dhcpv6_opts, ovs_be32 iaid)
{
while (userdata->size) {
struct dhcp_opt6_header *userdata_opt = ofpbuf_try_pull(
struct dhcpv6_opt_header *userdata_opt = ofpbuf_try_pull(
userdata, sizeof *userdata_opt);
if (!userdata_opt) {
return false;
}

size_t size = ntohs(userdata_opt->size);
size_t size = ntohs(userdata_opt->len);
uint8_t *userdata_opt_data = ofpbuf_try_pull(userdata, size);
if (!userdata_opt_data) {
return false;
}

switch (ntohs(userdata_opt->opt_code)) {
switch (ntohs(userdata_opt->code)) {
case DHCPV6_OPT_SERVER_ID_CODE:
{
/* The Server Identifier option carries a DUID
Expand Down
10 changes: 5 additions & 5 deletions lib/actions.c
Expand Up @@ -2875,26 +2875,26 @@ static void
encode_put_dhcpv6_option(const struct ovnact_gen_option *o,
struct ofpbuf *ofpacts)
{
struct dhcp_opt6_header *opt = ofpbuf_put_uninit(ofpacts, sizeof *opt);
struct dhcpv6_opt_header *opt = ofpbuf_put_uninit(ofpacts, sizeof *opt);
const union expr_constant *c = o->value.values;
size_t n_values = o->value.n_values;
size_t size;

opt->opt_code = htons(o->option->code);
opt->code = htons(o->option->code);

if (!strcmp(o->option->type, "ipv6")) {
size = n_values * sizeof(struct in6_addr);
opt->size = htons(size);
opt->len = htons(size);
for (size_t i = 0; i < n_values; i++) {
ofpbuf_put(ofpacts, &c[i].value.ipv6, sizeof(struct in6_addr));
}
} else if (!strcmp(o->option->type, "mac")) {
size = sizeof(struct eth_addr);
opt->size = htons(size);
opt->len = htons(size);
ofpbuf_put(ofpacts, &c->value.mac, size);
} else if (!strcmp(o->option->type, "str")) {
size = strlen(c->string);
opt->size = htons(size);
opt->len = htons(size);
ofpbuf_put(ofpacts, c->string, size);
}
}
Expand Down
6 changes: 0 additions & 6 deletions lib/ovn-l7.h
Expand Up @@ -228,12 +228,6 @@ struct dhcp_opt_header {
#define DHCP_OPT_PAYLOAD(hdr) \
(void *)((char *)hdr + sizeof(struct dhcp_opt_header))

/* Used in the OpenFlow PACKET_IN userdata */
struct dhcp_opt6_header {
ovs_be16 opt_code;
ovs_be16 size;
};

/* These are not defined in ovs/lib/dhcp.h, hence defining here. */
#define OVN_DHCP_MSG_DECLINE 4
#define OVN_DHCP_MSG_RELEASE 7
Expand Down

0 comments on commit 57c788a

Please sign in to comment.