From owner-freebsd-arm@FreeBSD.ORG Thu Jan 24 15:50:17 2008 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 80A4C16A417; Thu, 24 Jan 2008 15:50:17 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 3110F13C467; Thu, 24 Jan 2008 15:50:17 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.1/8.14.1) with ESMTP id m0OFicOA010827; Thu, 24 Jan 2008 08:44:39 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 24 Jan 2008 08:48:28 -0700 (MST) Message-Id: <20080124.084828.1608359032.imp@bsdimp.com> To: wjw@digiware.nl From: "M. Warner Losh" In-Reply-To: <479880A7.1030107@digiware.nl> References: <20080118.145436.-1540399028.imp@bsdimp.com> <20080124114039.GF79134@zibbi.meraka.csir.co.za> <479880A7.1030107@digiware.nl> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-arm@freebsd.org, des@freebsd.org Subject: Re: sshd broken on arm? X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jan 2008 15:50:17 -0000 In message: <479880A7.1030107@digiware.nl> Willem Jan Withagen writes: : John Hay and others wrote: : >> : > : Hmmm Just to make sure that I'm on the right page. On FreeBSD ARM one : >> : > : is not supposed to be able to access unaligned memory? Ie. an int that : >> : > : does not start on an address that is a multiple of 4. : >> : > : : >> : > : In a C function if you have something like "char tmp[4]", can you assume : >> : > : that the compiler will align it on a 4 byte boundary or can it do it on : >> : > : a byte boundary? : >> : > : : >> : > : If one cannot access unaligned ints and char arrays are not int aligned, : >> : > : then we were just lucky that the code worked at some stage. : >> : > : >> : > You are correct. The fact that it seemed to work meant that we were : >> : > either getting lucky before, or there was some critical code on the : >> : > kernel side that has accidentally been removed... : : I was actually reading up on the Arm hardware when the first messages : came in. So in the continuance of reading I kept this question in the : back of my mind. And as far as I've now red and understood the : requirements of the ARM, they do not support unaligned memory accesses. : : I've also not seen any suggestions that they generate exceptions for : these arrors... Which would have been the nice thing to do. : : That holds for both 4-bytes ints, as for 2 byte words. : But it also hold for the code, "regular" code is 4 byte aligned and : thumb code is 2 byte aligned. And looking at the eg. indexing : instructions in the instructionset it is clear that the assumption above : is used in building the instructions. : : So here the adresses are not aligned because the variables before : occupie only 2 bytes in the first piece of code. And thus the suggested : ints in char tmp[*] are word aligned. : : The second part of the fix however has the first previous variable be: : int fd; Which would lead to 4-byte alignment and thus should not pose a : problem unless the compiler also reshuffles the order of the variables : on the stack. : : It gets even more funny that things work when the compiler is allowed to : optimise. : To be shure what is really going on, one would have to take a look at : the generated assembly code. : : And yes, I'm very amazed that if the above is the problem that it did : not raize it ugly head earlier.... : : > Ok, it turned out that the problem/change was not in the kernel. Old : > and new kernels work equally well/bad. On December 2, the default : > optimisation for the ARMs was changed from -O2 to -O. That is it. If : > I compile libsshd with -O2, it works and if I compile it with -O, it : > does not. : > : > Just adding __aligned(4) like Warner suggested also fix the problem. : > : > Index: crypto/openssh/monitor_fdpass.c : > =================================================================== : > RCS file: /home/ncvs/src/crypto/openssh/monitor_fdpass.c,v : > retrieving revision 1.1.1.7 : > diff -u -r1.1.1.7 monitor_fdpass.c : > --- crypto/openssh/monitor_fdpass.c 10 Nov 2006 16:38:34 -0000 1.1.1.7 : > +++ crypto/openssh/monitor_fdpass.c 24 Jan 2008 11:32:43 -0000 : > @@ -49,7 +49,7 @@ : > char ch = '\0'; : > ssize_t n; : > #ifndef HAVE_ACCRIGHTS_IN_MSGHDR : > - char tmp[CMSG_SPACE(sizeof(int))]; : > + char tmp[CMSG_SPACE(sizeof(int))] __aligned(4); : > struct cmsghdr *cmsg; : > #endif : > : > @@ -94,7 +94,7 @@ : > char ch; : > int fd; : > #ifndef HAVE_ACCRIGHTS_IN_MSGHDR : > - char tmp[CMSG_SPACE(sizeof(int))]; : > + char tmp[CMSG_SPACE(sizeof(int))] __aligned(4); : > struct cmsghdr *cmsg; : > #endif : > : > : > So what should we do? : : I would say that the correct fix would be to fix the compiler. : Going through all the code and make sure that everything is correctly : aligned would be a humongous task.... Actually, the above fix *IS* the correct fix for arm given the ABI that we're using... Warner