Date: Thu, 24 Jan 2008 08:48:28 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: wjw@digiware.nl Cc: freebsd-arm@freebsd.org, des@freebsd.org Subject: Re: sshd broken on arm? Message-ID: <20080124.084828.1608359032.imp@bsdimp.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <479880A7.1030107@digiware.nl>
Willem Jan Withagen <wjw@digiware.nl> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080124.084828.1608359032.imp>