aarch64 clone() man page omission

aarch64 clone() man page omission

[Colin Ian King]

Hi there,

currently, the aarch64 clone() system call requires the stack to be
aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
copy_thread():

                if (stack_start) {
                        if (is_compat_thread(task_thread_info(p)))
                                childregs->compat_sp = stack_start;
                        /* 16-byte aligned stack mandatory on AArch64 */
                        else if (stack_start & 15)
                                return -EINVAL;
                        else
                                childregs->sp = stack_start;
                }


..and returns -EINVAL if not aligned correctly.  This should be added to
the manual page clone(2) as it took me a while to figure out why clone()
was failing with -EINVAL for aarch64 but not on x86.

Colin
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Michael Kerrisk (man-pages)]

Hi Colin,

On 04/25/2016 09:42 PM, Colin Ian King wrote:
> Hi there,
> 
> currently, the aarch64 clone() system call requires the stack to be
> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> copy_thread():
> 
>                 if (stack_start) {
>                         if (is_compat_thread(task_thread_info(p)))
>                                 childregs->compat_sp = stack_start;
>                         /* 16-byte aligned stack mandatory on AArch64 */
>                         else if (stack_start & 15)
>                                 return -EINVAL;
>                         else
>                                 childregs->sp = stack_start;
>                 }
> 
> 
> ..and returns -EINVAL if not aligned correctly.  This should be added to
> the manual page clone(2) as it took me a while to figure out why clone()
> was failing with -EINVAL for aarch64 but not on x86.

Thanks for the report!I applied the patch below. (Seem okay?)

Cheers,

Michael

diff --git a/man2/clone.2 b/man2/clone.2
index c9ce21c..184875a 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -969,6 +969,13 @@ but the kernel was not configured with the
 .B CONFIG_UTS
 option.
 .TP
+.B EINVAL
+.I child_stack
+is not aligned to a suitable boundary for this architecture.
+For example, on aarch64,
+.I child_stack
+must be a multiple of 16.
+.TP
 .B ENOMEM
 Cannot allocate sufficient memory to allocate a task structure for the
 child, or to copy those parts of the caller's context that need to be


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

On 25 Apr 2016 20:42, Colin Ian King wrote:
> currently, the aarch64 clone() system call requires the stack to be
> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> copy_thread():
> 
>                 if (stack_start) {
>                         if (is_compat_thread(task_thread_info(p)))
>                                 childregs->compat_sp = stack_start;
>                         /* 16-byte aligned stack mandatory on AArch64 */
>                         else if (stack_start & 15)
>                                 return -EINVAL;
>                         else
>                                 childregs->sp = stack_start;
>                 }
> 
> 
> ..and returns -EINVAL if not aligned correctly.  This should be added to
> the manual page clone(2) as it took me a while to figure out why clone()
> was failing with -EINVAL for aarch64 but not on x86.

seems weird for the kernel to be enforcing this.  is it just because of
the stated ABI ?  or is there some weird requirement in the kernel itself
that requires this ?  it's not like other arches have this check, and
there are def ABI requirements about stack alignments in C.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: aarch64 clone() man page omission

[Colin Ian King]

[-- Attachment #1.1: Type: text/plain, Size: 1429 bytes --]

On 09/05/16 22:31, Mike Frysinger wrote:
> On 25 Apr 2016 20:42, Colin Ian King wrote:
>> currently, the aarch64 clone() system call requires the stack to be
>> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
>> copy_thread():
>>
>>                 if (stack_start) {
>>                         if (is_compat_thread(task_thread_info(p)))
>>                                 childregs->compat_sp = stack_start;
>>                         /* 16-byte aligned stack mandatory on AArch64 */
>>                         else if (stack_start & 15)
>>                                 return -EINVAL;
>>                         else
>>                                 childregs->sp = stack_start;
>>                 }
>>
>>
>> ..and returns -EINVAL if not aligned correctly.  This should be added to
>> the manual page clone(2) as it took me a while to figure out why clone()
>> was failing with -EINVAL for aarch64 but not on x86.
> 
> seems weird for the kernel to be enforcing this.  is it just because of
> the stated ABI ?  or is there some weird requirement in the kernel itself
> that requires this ?  it's not like other arches have this check, and
> there are def ABI requirements about stack alignments in C.
> -mike
> 
The article here indicates it is an aarch64 convention:

https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64





[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: aarch64 clone() man page omission

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]

On 09 May 2016 22:40, Colin Ian King wrote:
> On 09/05/16 22:31, Mike Frysinger wrote:
> > On 25 Apr 2016 20:42, Colin Ian King wrote:
> >> currently, the aarch64 clone() system call requires the stack to be
> >> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> >> copy_thread():
> >>
> >>                 if (stack_start) {
> >>                         if (is_compat_thread(task_thread_info(p)))
> >>                                 childregs->compat_sp = stack_start;
> >>                         /* 16-byte aligned stack mandatory on AArch64 */
> >>                         else if (stack_start & 15)
> >>                                 return -EINVAL;
> >>                         else
> >>                                 childregs->sp = stack_start;
> >>                 }
> >>
> >>
> >> ..and returns -EINVAL if not aligned correctly.  This should be added to
> >> the manual page clone(2) as it took me a while to figure out why clone()
> >> was failing with -EINVAL for aarch64 but not on x86.
> > 
> > seems weird for the kernel to be enforcing this.  is it just because of
> > the stated ABI ?  or is there some weird requirement in the kernel itself
> > that requires this ?  it's not like other arches have this check, and
> > there are def ABI requirements about stack alignments in C.
> 
> The article here indicates it is an aarch64 convention:
> 
> https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64

that checks my point about the ABI having alignment requirements, but
that doesn't mean it needs to be checked/enforced in the kernel.  all
the limitations i see there can be seen in other arches, but we don't
have those arches do any stack alignment checking.  so should we be
dropping it from aarch64 ?  why does it need to be special here ?
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: aarch64 clone() man page omission

[Catalin Marinas]

On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
> On 09 May 2016 22:40, Colin Ian King wrote:
> > On 09/05/16 22:31, Mike Frysinger wrote:
> > > On 25 Apr 2016 20:42, Colin Ian King wrote:
> > >> currently, the aarch64 clone() system call requires the stack to be
> > >> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> > >> copy_thread():
> > >>
> > >>                 if (stack_start) {
> > >>                         if (is_compat_thread(task_thread_info(p)))
> > >>                                 childregs->compat_sp = stack_start;
> > >>                         /* 16-byte aligned stack mandatory on AArch64 */
> > >>                         else if (stack_start & 15)
> > >>                                 return -EINVAL;
> > >>                         else
> > >>                                 childregs->sp = stack_start;
> > >>                 }
> > >>
> > >>
> > >> ..and returns -EINVAL if not aligned correctly.  This should be added to
> > >> the manual page clone(2) as it took me a while to figure out why clone()
> > >> was failing with -EINVAL for aarch64 but not on x86.
> > > 
> > > seems weird for the kernel to be enforcing this.  is it just because of
> > > the stated ABI ?  or is there some weird requirement in the kernel itself
> > > that requires this ?  it's not like other arches have this check, and
> > > there are def ABI requirements about stack alignments in C.
> > 
> > The article here indicates it is an aarch64 convention:
> > 
> > https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
> 
> that checks my point about the ABI having alignment requirements, but
> that doesn't mean it needs to be checked/enforced in the kernel.  all
> the limitations i see there can be seen in other arches, but we don't
> have those arches do any stack alignment checking.  so should we be
> dropping it from aarch64 ?  why does it need to be special here ?

It is not just a software ABI requirement but a hardware one. If you try
to access the stack with an unaligned SP value, you get a fault followed
by a SIGBUS delivered to the user application. We decided to enforce
this at the copy_thread() level, it is easier to catch such issue early
than debugging SIGBUS delivered to a thread.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Colin Ian King]

On 11/05/16 14:18, Catalin Marinas wrote:
> On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
>> On 09 May 2016 22:40, Colin Ian King wrote:
>>> On 09/05/16 22:31, Mike Frysinger wrote:
>>>> On 25 Apr 2016 20:42, Colin Ian King wrote:
>>>>> currently, the aarch64 clone() system call requires the stack to be
>>>>> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
>>>>> copy_thread():
>>>>>
>>>>>                 if (stack_start) {
>>>>>                         if (is_compat_thread(task_thread_info(p)))
>>>>>                                 childregs->compat_sp = stack_start;
>>>>>                         /* 16-byte aligned stack mandatory on AArch64 */
>>>>>                         else if (stack_start & 15)
>>>>>                                 return -EINVAL;
>>>>>                         else
>>>>>                                 childregs->sp = stack_start;
>>>>>                 }
>>>>>
>>>>>
>>>>> ..and returns -EINVAL if not aligned correctly.  This should be added to
>>>>> the manual page clone(2) as it took me a while to figure out why clone()
>>>>> was failing with -EINVAL for aarch64 but not on x86.
>>>>
>>>> seems weird for the kernel to be enforcing this.  is it just because of
>>>> the stated ABI ?  or is there some weird requirement in the kernel itself
>>>> that requires this ?  it's not like other arches have this check, and
>>>> there are def ABI requirements about stack alignments in C.
>>>
>>> The article here indicates it is an aarch64 convention:
>>>
>>> https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
>>
>> that checks my point about the ABI having alignment requirements, but
>> that doesn't mean it needs to be checked/enforced in the kernel.  all
>> the limitations i see there can be seen in other arches, but we don't
>> have those arches do any stack alignment checking.  so should we be
>> dropping it from aarch64 ?  why does it need to be special here ?
> 
> It is not just a software ABI requirement but a hardware one. If you try
> to access the stack with an unaligned SP value, you get a fault followed
> by a SIGBUS delivered to the user application. We decided to enforce
> this at the copy_thread() level, it is easier to catch such issue early
> than debugging SIGBUS delivered to a thread.
> 
Rather than returning -EINVAL would it be more useful re-align
stack_start to the 16 byte boundary in copy_thread as a silent but
useful fixup?  It took me a while to debug the -EINVAL on the clone()
system call to figure out what was wrong because I didn't realize
aarch64 has this constraint.

Colin
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Catalin Marinas]

On Wed, May 11, 2016 at 02:33:01PM +0100, Colin Ian King wrote:
> On 11/05/16 14:18, Catalin Marinas wrote:
> > On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
> >> On 09 May 2016 22:40, Colin Ian King wrote:
> >>> On 09/05/16 22:31, Mike Frysinger wrote:
> >>>> On 25 Apr 2016 20:42, Colin Ian King wrote:
> >>>>> currently, the aarch64 clone() system call requires the stack to be
> >>>>> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> >>>>> copy_thread():
> >>>>>
> >>>>>                 if (stack_start) {
> >>>>>                         if (is_compat_thread(task_thread_info(p)))
> >>>>>                                 childregs->compat_sp = stack_start;
> >>>>>                         /* 16-byte aligned stack mandatory on AArch64 */
> >>>>>                         else if (stack_start & 15)
> >>>>>                                 return -EINVAL;
> >>>>>                         else
> >>>>>                                 childregs->sp = stack_start;
> >>>>>                 }
> >>>>>
> >>>>>
> >>>>> ..and returns -EINVAL if not aligned correctly.  This should be added to
> >>>>> the manual page clone(2) as it took me a while to figure out why clone()
> >>>>> was failing with -EINVAL for aarch64 but not on x86.
> >>>>
> >>>> seems weird for the kernel to be enforcing this.  is it just because of
> >>>> the stated ABI ?  or is there some weird requirement in the kernel itself
> >>>> that requires this ?  it's not like other arches have this check, and
> >>>> there are def ABI requirements about stack alignments in C.
> >>>
> >>> The article here indicates it is an aarch64 convention:
> >>>
> >>> https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
> >>
> >> that checks my point about the ABI having alignment requirements, but
> >> that doesn't mean it needs to be checked/enforced in the kernel.  all
> >> the limitations i see there can be seen in other arches, but we don't
> >> have those arches do any stack alignment checking.  so should we be
> >> dropping it from aarch64 ?  why does it need to be special here ?
> > 
> > It is not just a software ABI requirement but a hardware one. If you try
> > to access the stack with an unaligned SP value, you get a fault followed
> > by a SIGBUS delivered to the user application. We decided to enforce
> > this at the copy_thread() level, it is easier to catch such issue early
> > than debugging SIGBUS delivered to a thread.
> 
> Rather than returning -EINVAL would it be more useful re-align
> stack_start to the 16 byte boundary in copy_thread as a silent but
> useful fixup?

I wouldn't silently re-align the stack, it's a significant kernel ABI
change. Even dropping -EINVAL in favour of a later SIGBUS is an ABI
change, though not sure if any user apps or libraries would be affected
(I wouldn't expect them to rely on the -EINVAL return).

It seems that musl does this alignment in its clone(2) implementation:

https://git.musl-libc.org/cgit/musl/tree/src/thread/aarch64/clone.s

IIUC, glibc does not.

> It took me a while to debug the -EINVAL on the clone() system call to
> figure out what was wrong because I didn't realize aarch64 has this
> constraint.

Would it have been easier to get a SIGBUS on the first stack access?

It's worth posting a patch removing -EINVAL on linux-arm-kernel for
wider discussion.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Colin Ian King]

On 11/05/16 16:22, Catalin Marinas wrote:
> On Wed, May 11, 2016 at 02:33:01PM +0100, Colin Ian King wrote:
>> On 11/05/16 14:18, Catalin Marinas wrote:
>>> On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
>>>> On 09 May 2016 22:40, Colin Ian King wrote:
>>>>> On 09/05/16 22:31, Mike Frysinger wrote:
>>>>>> On 25 Apr 2016 20:42, Colin Ian King wrote:
>>>>>>> currently, the aarch64 clone() system call requires the stack to be
>>>>>>> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
>>>>>>> copy_thread():
>>>>>>>
>>>>>>>                 if (stack_start) {
>>>>>>>                         if (is_compat_thread(task_thread_info(p)))
>>>>>>>                                 childregs->compat_sp = stack_start;
>>>>>>>                         /* 16-byte aligned stack mandatory on AArch64 */
>>>>>>>                         else if (stack_start & 15)
>>>>>>>                                 return -EINVAL;
>>>>>>>                         else
>>>>>>>                                 childregs->sp = stack_start;
>>>>>>>                 }
>>>>>>>
>>>>>>>
>>>>>>> ..and returns -EINVAL if not aligned correctly.  This should be added to
>>>>>>> the manual page clone(2) as it took me a while to figure out why clone()
>>>>>>> was failing with -EINVAL for aarch64 but not on x86.
>>>>>>
>>>>>> seems weird for the kernel to be enforcing this.  is it just because of
>>>>>> the stated ABI ?  or is there some weird requirement in the kernel itself
>>>>>> that requires this ?  it's not like other arches have this check, and
>>>>>> there are def ABI requirements about stack alignments in C.
>>>>>
>>>>> The article here indicates it is an aarch64 convention:
>>>>>
>>>>> https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
>>>>
>>>> that checks my point about the ABI having alignment requirements, but
>>>> that doesn't mean it needs to be checked/enforced in the kernel.  all
>>>> the limitations i see there can be seen in other arches, but we don't
>>>> have those arches do any stack alignment checking.  so should we be
>>>> dropping it from aarch64 ?  why does it need to be special here ?
>>>
>>> It is not just a software ABI requirement but a hardware one. If you try
>>> to access the stack with an unaligned SP value, you get a fault followed
>>> by a SIGBUS delivered to the user application. We decided to enforce
>>> this at the copy_thread() level, it is easier to catch such issue early
>>> than debugging SIGBUS delivered to a thread.
>>
>> Rather than returning -EINVAL would it be more useful re-align
>> stack_start to the 16 byte boundary in copy_thread as a silent but
>> useful fixup?
> 
> I wouldn't silently re-align the stack, it's a significant kernel ABI
> change. Even dropping -EINVAL in favour of a later SIGBUS is an ABI
> change, though not sure if any user apps or libraries would be affected
> (I wouldn't expect them to rely on the -EINVAL return).
> 
> It seems that musl does this alignment in its clone(2) implementation:
> 
> https://git.musl-libc.org/cgit/musl/tree/src/thread/aarch64/clone.s
> 
> IIUC, glibc does not.
> 
>> It took me a while to debug the -EINVAL on the clone() system call to
>> figure out what was wrong because I didn't realize aarch64 has this
>> constraint.
> 
> Would it have been easier to get a SIGBUS on the first stack access?

Not sure if that's a rhetorical question, but needless to say, a SIGBUS
on the stack would be more of a hint from userspace that can be debugged
without diving into the kernel than having -EINVAL IMHO.
> 
> It's worth posting a patch removing -EINVAL on linux-arm-kernel for
> wider discussion.
> 
Yup, good idea.

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2926 bytes --]

On 11 May 2016 14:18, Catalin Marinas wrote:
> On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
> > On 09 May 2016 22:40, Colin Ian King wrote:
> > > On 09/05/16 22:31, Mike Frysinger wrote:
> > > > On 25 Apr 2016 20:42, Colin Ian King wrote:
> > > >> currently, the aarch64 clone() system call requires the stack to be
> > > >> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> > > >> copy_thread():
> > > >>
> > > >>                 if (stack_start) {
> > > >>                         if (is_compat_thread(task_thread_info(p)))
> > > >>                                 childregs->compat_sp = stack_start;
> > > >>                         /* 16-byte aligned stack mandatory on AArch64 */
> > > >>                         else if (stack_start & 15)
> > > >>                                 return -EINVAL;
> > > >>                         else
> > > >>                                 childregs->sp = stack_start;
> > > >>                 }
> > > >>
> > > >>
> > > >> ..and returns -EINVAL if not aligned correctly.  This should be added to
> > > >> the manual page clone(2) as it took me a while to figure out why clone()
> > > >> was failing with -EINVAL for aarch64 but not on x86.
> > > > 
> > > > seems weird for the kernel to be enforcing this.  is it just because of
> > > > the stated ABI ?  or is there some weird requirement in the kernel itself
> > > > that requires this ?  it's not like other arches have this check, and
> > > > there are def ABI requirements about stack alignments in C.
> > > 
> > > The article here indicates it is an aarch64 convention:
> > > 
> > > https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
> > 
> > that checks my point about the ABI having alignment requirements, but
> > that doesn't mean it needs to be checked/enforced in the kernel.  all
> > the limitations i see there can be seen in other arches, but we don't
> > have those arches do any stack alignment checking.  so should we be
> > dropping it from aarch64 ?  why does it need to be special here ?
> 
> It is not just a software ABI requirement but a hardware one. If you try
> to access the stack with an unaligned SP value, you get a fault followed
> by a SIGBUS delivered to the user application. We decided to enforce
> this at the copy_thread() level, it is easier to catch such issue early
> than debugging SIGBUS delivered to a thread.

as i said, that same behavior can be observed on other arches.  i know of
at least one for sure that if the stack is unaligned, then push/pop ops
will also trigger SIGBUS.  x86 tends to be more forgiving, but if it isn't
16bytes, then it is known that SSE optimized code will often fault.

so the question is still: why is aarch64 enforcing in the kernel what all
other arches have left alone even when they behave the same in hardware ?
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: aarch64 clone() man page omission

[Catalin Marinas]

On Wed, May 11, 2016 at 10:00:24AM -0400, Mike Frysinger wrote:
> On 11 May 2016 14:18, Catalin Marinas wrote:
> > On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
> > > On 09 May 2016 22:40, Colin Ian King wrote:
> > > > On 09/05/16 22:31, Mike Frysinger wrote:
> > > > > On 25 Apr 2016 20:42, Colin Ian King wrote:
> > > > >> currently, the aarch64 clone() system call requires the stack to be
> > > > >> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> > > > >> copy_thread():
> > > > >>
> > > > >>                 if (stack_start) {
> > > > >>                         if (is_compat_thread(task_thread_info(p)))
> > > > >>                                 childregs->compat_sp = stack_start;
> > > > >>                         /* 16-byte aligned stack mandatory on AArch64 */
> > > > >>                         else if (stack_start & 15)
> > > > >>                                 return -EINVAL;
> > > > >>                         else
> > > > >>                                 childregs->sp = stack_start;
> > > > >>                 }
> > > > >>
> > > > >>
> > > > >> ..and returns -EINVAL if not aligned correctly.  This should be added to
> > > > >> the manual page clone(2) as it took me a while to figure out why clone()
> > > > >> was failing with -EINVAL for aarch64 but not on x86.
> > > > > 
> > > > > seems weird for the kernel to be enforcing this.  is it just because of
> > > > > the stated ABI ?  or is there some weird requirement in the kernel itself
> > > > > that requires this ?  it's not like other arches have this check, and
> > > > > there are def ABI requirements about stack alignments in C.
> > > > 
> > > > The article here indicates it is an aarch64 convention:
> > > > 
> > > > https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
> > > 
> > > that checks my point about the ABI having alignment requirements, but
> > > that doesn't mean it needs to be checked/enforced in the kernel.  all
> > > the limitations i see there can be seen in other arches, but we don't
> > > have those arches do any stack alignment checking.  so should we be
> > > dropping it from aarch64 ?  why does it need to be special here ?
> > 
> > It is not just a software ABI requirement but a hardware one. If you try
> > to access the stack with an unaligned SP value, you get a fault followed
> > by a SIGBUS delivered to the user application. We decided to enforce
> > this at the copy_thread() level, it is easier to catch such issue early
> > than debugging SIGBUS delivered to a thread.
> 
> as i said, that same behavior can be observed on other arches.  i know of
> at least one for sure that if the stack is unaligned, then push/pop ops
> will also trigger SIGBUS.  x86 tends to be more forgiving, but if it isn't
> 16bytes, then it is known that SSE optimized code will often fault.
> 
> so the question is still: why is aarch64 enforcing in the kernel what all
> other arches have left alone even when they behave the same in hardware ?

This was an early decision before we upstreamed the AArch64 kernel
patches. Whether it was right or not it doesn't matter much now; at this
point it is considered kernel-user (syscall) ABI and any change would
require careful review.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: aarch64 clone() man page omission

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 3802 bytes --]

On 11 May 2016 16:26, Catalin Marinas wrote:
> On Wed, May 11, 2016 at 10:00:24AM -0400, Mike Frysinger wrote:
> > On 11 May 2016 14:18, Catalin Marinas wrote:
> > > On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
> > > > On 09 May 2016 22:40, Colin Ian King wrote:
> > > > > On 09/05/16 22:31, Mike Frysinger wrote:
> > > > > > On 25 Apr 2016 20:42, Colin Ian King wrote:
> > > > > >> currently, the aarch64 clone() system call requires the stack to be
> > > > > >> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> > > > > >> copy_thread():
> > > > > >>
> > > > > >>                 if (stack_start) {
> > > > > >>                         if (is_compat_thread(task_thread_info(p)))
> > > > > >>                                 childregs->compat_sp = stack_start;
> > > > > >>                         /* 16-byte aligned stack mandatory on AArch64 */
> > > > > >>                         else if (stack_start & 15)
> > > > > >>                                 return -EINVAL;
> > > > > >>                         else
> > > > > >>                                 childregs->sp = stack_start;
> > > > > >>                 }
> > > > > >>
> > > > > >>
> > > > > >> ..and returns -EINVAL if not aligned correctly.  This should be added to
> > > > > >> the manual page clone(2) as it took me a while to figure out why clone()
> > > > > >> was failing with -EINVAL for aarch64 but not on x86.
> > > > > > 
> > > > > > seems weird for the kernel to be enforcing this.  is it just because of
> > > > > > the stated ABI ?  or is there some weird requirement in the kernel itself
> > > > > > that requires this ?  it's not like other arches have this check, and
> > > > > > there are def ABI requirements about stack alignments in C.
> > > > > 
> > > > > The article here indicates it is an aarch64 convention:
> > > > > 
> > > > > https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
> > > > 
> > > > that checks my point about the ABI having alignment requirements, but
> > > > that doesn't mean it needs to be checked/enforced in the kernel.  all
> > > > the limitations i see there can be seen in other arches, but we don't
> > > > have those arches do any stack alignment checking.  so should we be
> > > > dropping it from aarch64 ?  why does it need to be special here ?
> > > 
> > > It is not just a software ABI requirement but a hardware one. If you try
> > > to access the stack with an unaligned SP value, you get a fault followed
> > > by a SIGBUS delivered to the user application. We decided to enforce
> > > this at the copy_thread() level, it is easier to catch such issue early
> > > than debugging SIGBUS delivered to a thread.
> > 
> > as i said, that same behavior can be observed on other arches.  i know of
> > at least one for sure that if the stack is unaligned, then push/pop ops
> > will also trigger SIGBUS.  x86 tends to be more forgiving, but if it isn't
> > 16bytes, then it is known that SSE optimized code will often fault.
> > 
> > so the question is still: why is aarch64 enforcing in the kernel what all
> > other arches have left alone even when they behave the same in hardware ?
> 
> This was an early decision before we upstreamed the AArch64 kernel
> patches. Whether it was right or not it doesn't matter much now;

the logic behind it still matters.  what was it ?  or was it just what
you outlined above ?

> at this
> point it is considered kernel-user (syscall) ABI and any change would
> require careful review.

i don't think this classifies as ABI: we're talking about relaxing a
restriction, not adding a new one.  if we delete this code, all valid
old binaries that worked in the past will continue to work.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: aarch64 clone() man page omission

[Catalin Marinas]

On Wed, May 11, 2016 at 12:27:51PM -0400, Mike Frysinger wrote:
> On 11 May 2016 16:26, Catalin Marinas wrote:
> > On Wed, May 11, 2016 at 10:00:24AM -0400, Mike Frysinger wrote:
> > > On 11 May 2016 14:18, Catalin Marinas wrote:
> > > > On Tue, May 10, 2016 at 10:50:40PM -0400, Mike Frysinger wrote:
> > > > > On 09 May 2016 22:40, Colin Ian King wrote:
> > > > > > On 09/05/16 22:31, Mike Frysinger wrote:
> > > > > > > On 25 Apr 2016 20:42, Colin Ian King wrote:
> > > > > > >> currently, the aarch64 clone() system call requires the stack to be
> > > > > > >> aligned at a 16 byte boundary, see arch/arm64/kernel/process.c,
> > > > > > >> copy_thread():
> > > > > > >>
> > > > > > >>                 if (stack_start) {
> > > > > > >>                         if (is_compat_thread(task_thread_info(p)))
> > > > > > >>                                 childregs->compat_sp = stack_start;
> > > > > > >>                         /* 16-byte aligned stack mandatory on AArch64 */
> > > > > > >>                         else if (stack_start & 15)
> > > > > > >>                                 return -EINVAL;
> > > > > > >>                         else
> > > > > > >>                                 childregs->sp = stack_start;
> > > > > > >>                 }
> > > > > > >>
> > > > > > >>
> > > > > > >> ..and returns -EINVAL if not aligned correctly.  This should be added to
> > > > > > >> the manual page clone(2) as it took me a while to figure out why clone()
> > > > > > >> was failing with -EINVAL for aarch64 but not on x86.
> > > > > > > 
> > > > > > > seems weird for the kernel to be enforcing this.  is it just because of
> > > > > > > the stated ABI ?  or is there some weird requirement in the kernel itself
> > > > > > > that requires this ?  it's not like other arches have this check, and
> > > > > > > there are def ABI requirements about stack alignments in C.
> > > > > > 
> > > > > > The article here indicates it is an aarch64 convention:
> > > > > > 
> > > > > > https://community.arm.com/groups/processors/blog/2015/11/19/using-the-stack-in-aarch32-and-aarch64
> > > > > 
> > > > > that checks my point about the ABI having alignment requirements, but
> > > > > that doesn't mean it needs to be checked/enforced in the kernel.  all
> > > > > the limitations i see there can be seen in other arches, but we don't
> > > > > have those arches do any stack alignment checking.  so should we be
> > > > > dropping it from aarch64 ?  why does it need to be special here ?
> > > > 
> > > > It is not just a software ABI requirement but a hardware one. If you try
> > > > to access the stack with an unaligned SP value, you get a fault followed
> > > > by a SIGBUS delivered to the user application. We decided to enforce
> > > > this at the copy_thread() level, it is easier to catch such issue early
> > > > than debugging SIGBUS delivered to a thread.
> > > 
> > > as i said, that same behavior can be observed on other arches.  i know of
> > > at least one for sure that if the stack is unaligned, then push/pop ops
> > > will also trigger SIGBUS.  x86 tends to be more forgiving, but if it isn't
> > > 16bytes, then it is known that SSE optimized code will often fault.
> > > 
> > > so the question is still: why is aarch64 enforcing in the kernel what all
> > > other arches have left alone even when they behave the same in hardware ?
> > 
> > This was an early decision before we upstreamed the AArch64 kernel
> > patches. Whether it was right or not it doesn't matter much now;
> 
> the logic behind it still matters.  what was it ?  or was it just what
> you outlined above ?

I don't think there was much thought in that decisions ;). It's possible
that we wanted it to match the sys_rt_sigreturn() check on SP alignment
(though this one forces a SIGSEGV on a bad frame since this syscall can
never return).

> > at this point it is considered kernel-user (syscall) ABI and any
> > change would require careful review.
> 
> i don't think this classifies as ABI: we're talking about relaxing a
> restriction, not adding a new one.  if we delete this code, all valid
> old binaries that worked in the past will continue to work.

That's my assumption as well, I wouldn't expect any app to have relied
on -EINVAL for this special case. That's why I suggested to Colin that
he posted a patch on linux-arm-kernel and take it from there.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html