From owner-freebsd-net@FreeBSD.ORG Sat Dec 21 22:46:05 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org 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 E14B7A9; Sat, 21 Dec 2013 22:46:05 +0000 (UTC) Received: from mail-oa0-x22c.google.com (mail-oa0-x22c.google.com [IPv6:2607:f8b0:4003:c02::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 9A5DC1822; Sat, 21 Dec 2013 22:46:05 +0000 (UTC) Received: by mail-oa0-f44.google.com with SMTP id m1so4402781oag.3 for ; Sat, 21 Dec 2013 14:46:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=dzQ9RyTDate7VGewCv8xctEMpCXvuXrckqOWDSYrIRg=; b=FX5hLXFzrYzm4+DmdC6XI3vdyQ/JLWOXA0SvFgZuO0uyIOboNcj/YWE4bIafHaHtEd NVDwnOlGyPZ+mhpJdKz2VLWobijm3cvOHHR3FtYM7c5H6uav78Q8vG3SR0eLbe5ixphR Cru69Z3Rdh95bbNYcNVxD7NPQYhMTVuH/z9c9MZ6tT4+FNjPUHWFHNvC9S/9/y2Jk+Uv TdHHCRRWG3+5V9uqEA3X+Dmq5X6sd8NH+tzx+eWffSrTj2bXNPJXapVLxoL1kVOoyVjU Q/Tfo3iUTHuWgwhOCQDpsF7O30CD12bjNvyE7l85Fv7oK6meeTFdd3fTLGOLD0h6UaB6 /5nA== MIME-Version: 1.0 X-Received: by 10.60.35.73 with SMTP id f9mr12211155oej.50.1387665964913; Sat, 21 Dec 2013 14:46:04 -0800 (PST) Received: by 10.76.20.82 with HTTP; Sat, 21 Dec 2013 14:46:04 -0800 (PST) In-Reply-To: <20131221191552.GE99167@funkthat.com> References: <20131221191552.GE99167@funkthat.com> Date: Sun, 22 Dec 2013 00:46:04 +0200 Message-ID: Subject: Re: 10.0-RC1: net/mpd5 crashes in NgMkSockNode due to stack alignment on ARM EABI From: Guy Yur To: freebsd-arm@freebsd.org, freebsd-net@freebsd.org Content-Type: multipart/mixed; boundary=089e013c6fa0e84e3904ee132880 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Dec 2013 22:46:06 -0000 --089e013c6fa0e84e3904ee132880 Content-Type: text/plain; charset=UTF-8 On Sat, Dec 21, 2013 at 9:15 PM, John-Mark Gurney wrote: > Guy Yur wrote this message on Sat, Dec 21, 2013 at 19:24 +0200: >> I am running 10.0-RC1 on the BeagleBone Black and the net/mpd5 port is >> crashing in libnetgraph NgMkSockNode due to stack alignment. >> >> 10.0-RC1 World and kernel were compiled in a VirtualBox VM running >> 9.2-RELEASE-p2 i386. >> clang and ARM_EABI used as the default make options. >> >> Added prints in NgMkSockNode show rbuf is aligned on 2-byte and not >> 4-byte which is needed to access ni->id (a uint32_t). >> >> ni = 0xbfffe87a >> rbuf = 0xbfffe842 >> sizeof(resp->header) = 56 >> >> >> (gdb) bt >> #0 0x201529a0 in NgMkSockNode (name=, csp=0xbfffe95c, >> dsp=0xbfffe958) at /usr/src/lib/libnetgraph/sock.c:134 >> #1 0x00037b9c in MppcTestCap () at ccp_mppc.c:754 >> #2 0x0007c1f4 in main (ac=4, av=0xbfffeb90) at main.c:248 >> #3 0x0000d1b0 in __start (argc=4, argv=0xbfffeb90, env=0xbfffeba4, >> ps_strings=, obj=, >> cleanup=) at /usr/src/lib/csu/arm/crt1.c:115 >> #4 0x203e9dc0 in _thr_ast (curthread=0x200fd000) >> at /usr/src/lib/libthr/thread/thr_sig.c:265 >> >> >> Putting rbuf in a union with struct ng_mesg sorted the alignment to >> 4-byte and mpd5 didn't crash. >> I attached the changes I used to test mpd5 doesn't crash with correct alignment. > > The patch looks correct, but lets make sure that the -net people don't > have an issue with it... > > I've reattached Guy's patch for review. > > Guy, bug me in a week or so if I haven't committed it, and I will... > > Thanks for tracking this down. > > -- > John-Mark Gurney Voice: +1 415 225 5579 > > "All that I will do, has been done, All that I have, has not." Hi John, Should I still file a PR? 1. I noticed my patch causes a style bug with the rbuf line now taking 87 columns. 2. Since the union now has a ng_mesg struct, the casting to resp can be skipped and the union member used directly. Attached new patch breaking the rbuf line and swapping resp usage with res.res and &res.res Maybe a different name than res should be used for the union or the member. I only tested the new patch compiles for arm.armv6, haven't verified it. Thanks, Guy --089e013c6fa0e84e3904ee132880 Content-Type: application/octet-stream; name="sock-NgMkSockNode-2.patch" Content-Disposition: attachment; filename="sock-NgMkSockNode-2.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_hphga6do1 SW5kZXg6IGxpYi9saWJuZXRncmFwaC9zb2NrLmMKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbGliL2xpYm5ldGdy YXBoL3NvY2suYwkocmV2aXNpb24gMjU5MjUwKQorKysgbGliL2xpYm5ldGdyYXBoL3NvY2suYwko d29ya2luZyBjb3B5KQpAQCAtMTExLDkgKzExMSwxMiBAQAogCQkvKiBTYXZlIG5vZGUgbmFtZSAq LwogCQlzdHJsY3B5KG5hbWVidWYsIG5hbWUsIHNpemVvZihuYW1lYnVmKSk7CiAJfSBlbHNlIGlm IChkc3AgIT0gTlVMTCkgewotCQl1X2NoYXIgcmJ1ZltzaXplb2Yoc3RydWN0IG5nX21lc2cpICsg c2l6ZW9mKHN0cnVjdCBub2RlaW5mbyldOwotCQlzdHJ1Y3QgbmdfbWVzZyAqY29uc3QgcmVzcCA9 IChzdHJ1Y3QgbmdfbWVzZyAqKSByYnVmOwotCQlzdHJ1Y3Qgbm9kZWluZm8gKmNvbnN0IG5pID0g KHN0cnVjdCBub2RlaW5mbyAqKSByZXNwLT5kYXRhOworCQl1bmlvbiB7CisJCQl1X2NoYXIgcmJ1 ZltzaXplb2Yoc3RydWN0IG5nX21lc2cpICsKKwkJCSAgICBzaXplb2Yoc3RydWN0IG5vZGVpbmZv KV07CisJCQlzdHJ1Y3QgbmdfbWVzZyByZXM7CisJCX0gcmVzOworCQlzdHJ1Y3Qgbm9kZWluZm8g KmNvbnN0IG5pID0gKHN0cnVjdCBub2RlaW5mbyAqKSByZXMucmVzLmRhdGE7CiAKIAkJLyogRmlu ZCBvdXQgdGhlIG5vZGUgSUQgKi8KIAkJaWYgKE5nU2VuZE1zZyhjcywgIi4iLCBOR01fR0VORVJJ Q19DT09LSUUsCkBAIC0xMjMsNyArMTI2LDcgQEAKIAkJCQlOR0xPRygic2VuZCBub2RlaW5mbyIp OwogCQkJZ290byBlcnJvdXQ7CiAJCX0KLQkJaWYgKE5nUmVjdk1zZyhjcywgcmVzcCwgc2l6ZW9m KHJidWYpLCBOVUxMKSA8IDApIHsKKwkJaWYgKE5nUmVjdk1zZyhjcywgJnJlcy5yZXMsIHNpemVv ZihyZXMucmJ1ZiksIE5VTEwpIDwgMCkgewogCQkJZXJybm9zdiA9IGVycm5vOwogCQkJaWYgKF9n TmdEZWJ1Z0xldmVsID49IDEpCiAJCQkJTkdMT0coInJlY3Ygbm9kZWluZm8iKTsK --089e013c6fa0e84e3904ee132880--