sshd broken on arm?

[Dag-Erling Smørgrav]

Willem Jan Withagen <wjw at digiware.nl> writes:

> John Hay wrote:
>>>> The problem is that the char array isn't guaranteed to be aligned in
>>>> any way.  The fix posted is correct.
>>>>
>>>> There may be other fixes too, such as using a union to force
>>>> alignment.
>>> Well I'm sort of puzzled right now since after preprocessing the
>>> variable allocation part boils down to:
>>> =====
>>>  struct msghdr msg;
>>>  struct iovec vec;
>>>  char ch = '\0';
>>>  ssize_t n;
>>>
>>>  char tmp[((((unsigned)(sizeof(struct cmsghdr)) + (sizeof(int) -
>>> 1)) & ~(sizeof(int) - 1)) + (((unsigned)(sizeof(int)) + (sizeof(int
>>> ) - 1)) & ~(sizeof(int) - 1)))];
>>>  struct cmsghdr *cmsg;
>>> =====
>>> So as far as I can see is char tmp[] included between 2 4-byte
>>> items and  allocation should be "automagically" 4-byte aligned.
>>>
>>> Now adding simple code like tmp[0] = 50, the first part of the
>>> assembly is: (Comments are mine for as far as I can grasp them)
>>
>> Just doing tmp[0] = 50 will cause a byte access which should not be a
>> problem. The original code does something like this (simplified):
>>
>> char tmp[CMSG_SPACE(sizeof(int))];
>> int *ti;
>>
>> ti = tmp;
>> *ti = 50;
>>
>> Now the 50 is an int and not a byte and then the alignment does matter.
>
> I know, But to figure out where the array temp is allocated on the
> stack the easiest to do that is to assign a value to its first
> element. In assembly you will then find that the starting address that
> the compiler has calculated for this array.
> [...]

None of this matters.  What John pointed out means that the code is
wrong and the compiler is right.  The code is not allowed to assume that
an object is correctly aligned unless it is of a type that requires the
correct alignment.  The easiest way to do this is with a union, e.g.

union {
    char space[CMSG_SPACE(sizeof(int));
    int value;
} tmp;

tmp.value = 50;

A trickier way to do it is with a struct:

struct {
    int value;
    char pad[CMSG_SPACE(sizeof(int)) - sizeof(int)];
} tmp;

tmp.value = 50;

in both cases, you are guaranteed that 1) tmp is be large enough
(though possibly larger than required) 2) tmp is correctly aligned and
3) (void *)&tmp == (void *)&tmp.value.

DES
-- 
Dag-Erling Smørgrav - des at des.no