Date: Sat, 12 Apr 2014 16:03:38 +0300 From: Guy Yur <guyyur@gmail.com> To: FreeBSD-gnats-submit@freebsd.org Subject: arm/188510: [patch] "rtadvctl show" crashes on BeagleBone Black due to unaligned access Message-ID: <534939c2.045f0e0a.59a8.ffff808b@mx.google.com> Resent-Message-ID: <201404121310.s3CDA1sw056001@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 188510
>Category: arm
>Synopsis: [patch] "rtadvctl show" crashes on BeagleBone Black due to unaligned access
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-arm
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sat Apr 12 13:10:01 UTC 2014
>Closed-Date:
>Last-Modified:
>Originator: Guy Yur
>Release: FreeBSD 11.0-CURRENT arm.armv6
>Organization:
>Environment:
System: FreeBSD bbb-router.localdomain 11.0-CURRENT FreeBSD 11.0-CURRENT #0 r264364M: Sat Apr 12 14:23:46 IDT 2014 root@vm4.localdomain:/usr/obj/arm.armv6/usr/src/sys/BBB-ROUTER arm
>Description:
"rtadvctl show" core dumps on Bus error when run on BeagleBone Black.
(gdb) bt
#0 cm_pl2bin (str=<value optimized out>, cp=<value optimized out>)
at /usr/src/usr.sbin/rtadvctl/../rtadvd/control.c:458
#1 0x0000a59c in action_plgeneric (action=<value optimized out>,
plstr=<value optimized out>, buf=0xbfffcd6c "\001")
at /usr/src/usr.sbin/rtadvctl/rtadvctl.c:264
#2 0x0000a3c8 in action_propget (argv=0xbffff2d1 "", cp=0xbfffedf0)
at /usr/src/usr.sbin/rtadvctl/rtadvctl.c:285
#3 0x00009354 in action_show (argc=<value optimized out>,
argv=<value optimized out>)
at /usr/src/usr.sbin/rtadvctl/rtadvctl.c:432
#4 0x00009184 in main (argc=<value optimized out>, argv=0xbffff2d1)
at /usr/src/usr.sbin/rtadvctl/rtadvctl.c:187
#5 0x00008fdc in __start (argc=2, argv=0xbffffb98, env=0xbffffba4,
ps_strings=<value optimized out>, obj=0x2003c000,
cleanup=<value optimized out>) at /usr/src/lib/csu/arm/crt1.c:115
#6 0x2001fd3c in _rtld_get_stack_prot () from /libexec/ld-elf.so.1
#7 0x2001fd3c in _rtld_get_stack_prot () from /libexec/ld-elf.so.1
Current language: auto; currently minimal
disassembly:
0x0000b0c4 <cm_pl2bin+368>: str r0, [r8]
info registers:
...
r8 0xbfffcd87 -1073754745
...
pc 0xb0c4 45252
The protocol between rtadvd and rtadvctl writes a size_t len
followed by a string for each of ifname, key and value. When
ifname or key is supplied and their length is not a multiple of 4
the write of the next field size_t len will be to an unaligned
address and a trap will be generated on the BeagleBone Black.
>How-To-Repeat:
Run "rtadvctl show" on an arm machine with trapping
for unaligned access enabled.
>Fix:
Attached two patches with different ways to resolve the problem.
1. rtadvd_control_align.patch
Round up the strings to align on sizeof(size_t).
Is there a round up macro that can be used instead of explicit calculation?
Requires using matching rtadvd and rtadvctl since the protocol changed.
2. rtadvd_control_packed.patch
Use __packed structure access for the size_t len so byte instructions
will be used to read/write the len on arm.
Protocol doesn't change so compatibility between old and
fixed rtadvd and rtadvctl is kept.
--- rtadvd_control_align.patch begins here ---
Index: usr.sbin/rtadvd/control.c
===================================================================
--- usr.sbin/rtadvd/control.c (revision 264366)
+++ usr.sbin/rtadvd/control.c (working copy)
@@ -362,7 +362,7 @@ cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
}
memcpy(cp->cp_ifname, p, len);
cp->cp_ifname[len] = '\0';
- p += len;
+ p += (len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
}
lenp = (size_t *)p;
@@ -377,7 +377,7 @@ cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
}
memcpy(cp->cp_key, p, len);
cp->cp_key[len] = '\0';
- p += len;
+ p += (len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
}
lenp = (size_t *)p;
@@ -402,20 +402,26 @@ cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
size_t
cm_pl2bin(char *str, struct ctrl_msg_pl *cp)
{
- size_t len;
+ size_t len, ifname_len, key_len, val_len;
size_t *lenp;
char *p;
struct ctrl_msg_hdr *cm;
len = sizeof(size_t);
- if (cp->cp_ifname != NULL)
- len += strlen(cp->cp_ifname);
+ if (cp->cp_ifname != NULL) {
+ ifname_len = strlen(cp->cp_ifname);
+ len += (ifname_len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
+ }
len += sizeof(size_t);
- if (cp->cp_key != NULL)
- len += strlen(cp->cp_key);
+ if (cp->cp_key != NULL) {
+ key_len = strlen(cp->cp_key);
+ len += (key_len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
+ }
len += sizeof(size_t);
- if (cp->cp_val != NULL && cp->cp_val_len > 0)
- len += cp->cp_val_len;
+ if (cp->cp_val != NULL && cp->cp_val_len > 0) {
+ val_len = cp->cp_val_len;
+ len += (val_len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
+ }
if (len > CM_MSG_MAXLEN - sizeof(*cm)) {
syslog(LOG_DEBUG, "<%s> msg too long (len=%zu)",
@@ -426,36 +432,36 @@ cm_pl2bin(char *str, struct ctrl_msg_pl *cp)
memset(str, 0, len);
p = str;
lenp = (size_t *)p;
-
+
if (cp->cp_ifname != NULL) {
- *lenp++ = strlen(cp->cp_ifname);
+ *lenp++ = ifname_len;
p = (char *)lenp;
- memcpy(p, cp->cp_ifname, strlen(cp->cp_ifname));
- p += strlen(cp->cp_ifname);
+ memcpy(p, cp->cp_ifname, ifname_len);
+ p += (ifname_len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
} else {
- *lenp++ = '\0';
+ *lenp++ = 0;
p = (char *)lenp;
}
lenp = (size_t *)p;
if (cp->cp_key != NULL) {
- *lenp++ = strlen(cp->cp_key);
+ *lenp++ = key_len;
p = (char *)lenp;
- memcpy(p, cp->cp_key, strlen(cp->cp_key));
- p += strlen(cp->cp_key);
+ memcpy(p, cp->cp_key, key_len);
+ p += (key_len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
} else {
- *lenp++ = '\0';
+ *lenp++ = 0;
p = (char *)lenp;
}
lenp = (size_t *)p;
if (cp->cp_val != NULL && cp->cp_val_len > 0) {
- *lenp++ = cp->cp_val_len;
+ *lenp++ = val_len;
p = (char *)lenp;
- memcpy(p, cp->cp_val, cp->cp_val_len);
- p += cp->cp_val_len;
+ memcpy(p, cp->cp_val, val_len);
+ p += (val_len + sizeof(size_t)) & ~(sizeof(size_t) - 1);
} else {
- *lenp++ = '\0';
+ *lenp++ = 0;
p = (char *)lenp;
}
--- rtadvd_control_align.patch ends here ---
--- rtadvd_control_packed.patch begins here ---
Index: usr.sbin/rtadvd/control.c
===================================================================
--- usr.sbin/rtadvd/control.c (revision 264366)
+++ usr.sbin/rtadvd/control.c (working copy)
@@ -343,7 +343,10 @@ struct ctrl_msg_pl *
cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
{
size_t len;
- size_t *lenp;
+ struct unaligned_len {
+ size_t len;
+ } __packed;
+ struct unaligned_len *lenp;
char *p;
memset(cp, 0, sizeof(*cp));
@@ -350,9 +353,9 @@ cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
p = str;
- lenp = (size_t *)p;
- len = *lenp++;
- p = (char *)lenp;
+ lenp = (struct unaligned_len *)p;
+ len = lenp->len;
+ p = (char *)(lenp + 1);
syslog(LOG_DEBUG, "<%s> len(ifname) = %zu", __func__, len);
if (len > 0) {
cp->cp_ifname = malloc(len + 1);
@@ -365,9 +368,9 @@ cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
p += len;
}
- lenp = (size_t *)p;
- len = *lenp++;
- p = (char *)lenp;
+ lenp = (struct unaligned_len *)p;
+ len = lenp->len;
+ p = (char *)(lenp + 1);
syslog(LOG_DEBUG, "<%s> len(key) = %zu", __func__, len);
if (len > 0) {
cp->cp_key = malloc(len + 1);
@@ -380,9 +383,9 @@ cm_bin2pl(char *str, struct ctrl_msg_pl *cp)
p += len;
}
- lenp = (size_t *)p;
- len = *lenp++;
- p = (char *)lenp;
+ lenp = (struct unaligned_len *)p;
+ len = lenp->len;
+ p = (char *)(lenp + 1);
syslog(LOG_DEBUG, "<%s> len(val) = %zu", __func__, len);
if (len > 0) {
cp->cp_val = malloc(len + 1);
@@ -403,7 +406,10 @@ size_t
cm_pl2bin(char *str, struct ctrl_msg_pl *cp)
{
size_t len;
- size_t *lenp;
+ struct unaligned_len {
+ size_t len;
+ } __packed;
+ struct unaligned_len *lenp;
char *p;
struct ctrl_msg_hdr *cm;
@@ -425,38 +431,38 @@ cm_pl2bin(char *str, struct ctrl_msg_pl *cp)
syslog(LOG_DEBUG, "<%s> msglen=%zu", __func__, len);
memset(str, 0, len);
p = str;
- lenp = (size_t *)p;
-
+ lenp = (struct unaligned_len *)p;
+
if (cp->cp_ifname != NULL) {
- *lenp++ = strlen(cp->cp_ifname);
- p = (char *)lenp;
+ lenp->len = strlen(cp->cp_ifname);
+ p = (char *)(lenp + 1);
memcpy(p, cp->cp_ifname, strlen(cp->cp_ifname));
p += strlen(cp->cp_ifname);
} else {
- *lenp++ = '\0';
- p = (char *)lenp;
+ lenp->len = 0;
+ p = (char *)(lenp + 1);
}
- lenp = (size_t *)p;
+ lenp = (struct unaligned_len *)p;
if (cp->cp_key != NULL) {
- *lenp++ = strlen(cp->cp_key);
- p = (char *)lenp;
+ lenp->len = strlen(cp->cp_key);
+ p = (char *)(lenp + 1);
memcpy(p, cp->cp_key, strlen(cp->cp_key));
p += strlen(cp->cp_key);
} else {
- *lenp++ = '\0';
- p = (char *)lenp;
+ lenp->len = 0;
+ p = (char *)(lenp + 1);
}
- lenp = (size_t *)p;
+ lenp = (struct unaligned_len *)p;
if (cp->cp_val != NULL && cp->cp_val_len > 0) {
- *lenp++ = cp->cp_val_len;
- p = (char *)lenp;
+ lenp->len = cp->cp_val_len;
+ p = (char *)(lenp + 1);
memcpy(p, cp->cp_val, cp->cp_val_len);
p += cp->cp_val_len;
} else {
- *lenp++ = '\0';
- p = (char *)lenp;
+ lenp->len = 0;
+ p = (char *)(lenp + 1);
}
return (len);
--- rtadvd_control_packed.patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?534939c2.045f0e0a.59a8.ffff808b>