From owner-freebsd-arm@FreeBSD.ORG Sat Apr 12 13:10:01 2014 Return-Path: Delivered-To: freebsd-arm@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id CC06A645 for ; Sat, 12 Apr 2014 13:10:01 +0000 (UTC) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A4A4515AF for ; Sat, 12 Apr 2014 13:10:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.8/8.14.8) with ESMTP id s3CDA17E056002 for ; Sat, 12 Apr 2014 13:10:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.8/8.14.8/Submit) id s3CDA1sw056001; Sat, 12 Apr 2014 13:10:01 GMT (envelope-from gnats) Resent-Date: Sat, 12 Apr 2014 13:10:01 GMT Resent-Message-Id: <201404121310.s3CDA1sw056001@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-arm@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Guy Yur Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5586244C for ; Sat, 12 Apr 2014 13:04:06 +0000 (UTC) Received: from mail-ee0-x229.google.com (mail-ee0-x229.google.com [IPv6:2a00:1450:4013:c00::229]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D4A1A1562 for ; Sat, 12 Apr 2014 13:04:05 +0000 (UTC) Received: by mail-ee0-f41.google.com with SMTP id t10so5010322eei.14 for ; Sat, 12 Apr 2014 06:04:03 -0700 (PDT) Received: from vm2.localdomain ([31.210.185.47]) by mx.google.com with ESMTPSA id o4sm25266726eef.20.2014.04.12.06.04.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 12 Apr 2014 06:04:02 -0700 (PDT) Received: by vm2.localdomain (sSMTP sendmail emulation); Sat, 12 Apr 2014 16:03:38 +0300 Message-Id: <534939c2.045f0e0a.59a8.ffff808b@mx.google.com> Date: Sat, 12 Apr 2014 16:03:38 +0300 From: Guy Yur To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.114 Subject: arm/188510: [patch] "rtadvctl show" crashes on BeagleBone Black due to unaligned access X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list Reply-To: Guy Yur List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Apr 2014 13:10:01 -0000 >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=, cp=) at /usr/src/usr.sbin/rtadvctl/../rtadvd/control.c:458 #1 0x0000a59c in action_plgeneric (action=, plstr=, 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=, argv=) at /usr/src/usr.sbin/rtadvctl/rtadvctl.c:432 #4 0x00009184 in main (argc=, argv=0xbffff2d1) at /usr/src/usr.sbin/rtadvctl/rtadvctl.c:187 #5 0x00008fdc in __start (argc=2, argv=0xbffffb98, env=0xbffffba4, ps_strings=, obj=0x2003c000, cleanup=) 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 : 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: