[PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Manjeet Pawar]

MINSIGSTKSZ and SIGSTKSZ for ARM64 are not correctly set in latest kernel.
This patch fixes this issue.

This issue is reported in LTP (testcase: sigaltstack02.c).
Testcase failed when sigaltstack() called with stack size "MINSIGSTKSZ - 1"
Since in Glibc-2.22, MINSIGSTKSZ is set to 5120 but in kernel
it is set to 2048 so testcase gets failed.

Testcase Output:
sigaltstack02 1  TPASS  :  stgaltstack() fails, Invalid Flag value,errno:22
sigaltstack02 2  TFAIL  :  sigaltstack() returned 0, expected -1,errno:12

Reported Issue in Glibc Bugzilla:
Bugfix in Glibc-2.22: [Bug 16850]
https://sourceware.org/bugzilla/show_bug.cgi?id=16850

Signed-off-by: Akhilesh Kumar <akhilesh.k@samsung.com>
Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
---
v1 -> Changes in uapi overall header
v2 -> Changes done in arm64 headers
v3 -> Changes done in both uapi & arm64 headers

 arch/arm64/include/uapi/asm/signal.h |    3 +++
 include/uapi/asm-generic/signal.h    |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/signal.h b/arch/arm64/include/uapi/asm/signal.h
index 8d1e723..991bf5d 100644
--- a/arch/arm64/include/uapi/asm/signal.h
+++ b/arch/arm64/include/uapi/asm/signal.h
@@ -19,6 +19,9 @@
 /* Required for AArch32 compatibility. */
 #define SA_RESTORER	0x04000000
 
+#define MINSIGSTKSZ 5120
+#define SIGSTKSZ    16384
+
 #include <asm-generic/signal.h>
 
 #endif
diff --git a/include/uapi/asm-generic/signal.h b/include/uapi/asm-generic/signal.h
index 9df61f1..b98b67b 100644
--- a/include/uapi/asm-generic/signal.h
+++ b/include/uapi/asm-generic/signal.h
@@ -80,8 +80,10 @@
  *	SA_RESTORER	0x04000000
  */
 
+#if !defined MINSIGSTKSZ || !defined SIGSTKSZ
 #define MINSIGSTKSZ	2048
 #define SIGSTKSZ	8192
+#endif
 
 #ifndef __ASSEMBLY__
 typedef struct {
-- 
1.7.9.5

Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Will Deacon]

On Fri, Oct 09, 2015 at 03:59:40PM +0530, Manjeet Pawar wrote:
> MINSIGSTKSZ and SIGSTKSZ for ARM64 are not correctly set in latest kernel.
> This patch fixes this issue.
> 
> This issue is reported in LTP (testcase: sigaltstack02.c).
> Testcase failed when sigaltstack() called with stack size "MINSIGSTKSZ - 1"
> Since in Glibc-2.22, MINSIGSTKSZ is set to 5120 but in kernel
> it is set to 2048 so testcase gets failed.
> 
> Testcase Output:
> sigaltstack02 1  TPASS  :  stgaltstack() fails, Invalid Flag value,errno:22
> sigaltstack02 2  TFAIL  :  sigaltstack() returned 0, expected -1,errno:12

I'm still unable to reproduce this failure. Is this with defconfig?

> Reported Issue in Glibc Bugzilla:
> Bugfix in Glibc-2.22: [Bug 16850]
> https://sourceware.org/bugzilla/show_bug.cgi?id=16850
> 
> Signed-off-by: Akhilesh Kumar <akhilesh.k-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Manjeet Pawar <manjeet.p-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Rohit Thapliyal <r.thapliyal-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> v1 -> Changes in uapi overall header
> v2 -> Changes done in arm64 headers
> v3 -> Changes done in both uapi & arm64 headers
> 
>  arch/arm64/include/uapi/asm/signal.h |    3 +++
>  include/uapi/asm-generic/signal.h    |    2 ++
>  2 files changed, 5 insertions(+)

Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Arnd: are you planning to take this via asm-generic, or shall I queue it
on the arm64 fixes branch?

Will

> diff --git a/arch/arm64/include/uapi/asm/signal.h b/arch/arm64/include/uapi/asm/signal.h
> index 8d1e723..991bf5d 100644
> --- a/arch/arm64/include/uapi/asm/signal.h
> +++ b/arch/arm64/include/uapi/asm/signal.h
> @@ -19,6 +19,9 @@
>  /* Required for AArch32 compatibility. */
>  #define SA_RESTORER	0x04000000
>  
> +#define MINSIGSTKSZ 5120
> +#define SIGSTKSZ    16384
> +
>  #include <asm-generic/signal.h>
>  
>  #endif
> diff --git a/include/uapi/asm-generic/signal.h b/include/uapi/asm-generic/signal.h
> index 9df61f1..b98b67b 100644
> --- a/include/uapi/asm-generic/signal.h
> +++ b/include/uapi/asm-generic/signal.h
> @@ -80,8 +80,10 @@
>   *	SA_RESTORER	0x04000000
>   */
>  
> +#if !defined MINSIGSTKSZ || !defined SIGSTKSZ
>  #define MINSIGSTKSZ	2048
>  #define SIGSTKSZ	8192
> +#endif
>  
>  #ifndef __ASSEMBLY__
>  typedef struct {
> -- 
> 1.7.9.5
> 

Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Arnd Bergmann]

On Friday 09 October 2015 11:33:52 Will Deacon wrote:

> Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> 
> Arnd: are you planning to take this via asm-generic, or shall I queue it
> on the arm64 fixes branch?
> 

Please merge it for arm64 with my

Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

	Arnd

Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Szabolcs Nagy]

* Will Deacon <will.deacon@arm.com> [2015-10-09 11:33:52 +0100]:

> On Fri, Oct 09, 2015 at 03:59:40PM +0530, Manjeet Pawar wrote:
> > MINSIGSTKSZ and SIGSTKSZ for ARM64 are not correctly set in latest kernel.
> > This patch fixes this issue.
> > 
> > This issue is reported in LTP (testcase: sigaltstack02.c).
> > Testcase failed when sigaltstack() called with stack size "MINSIGSTKSZ - 1"
> > Since in Glibc-2.22, MINSIGSTKSZ is set to 5120 but in kernel
> > it is set to 2048 so testcase gets failed.
> > 
> > Testcase Output:
> > sigaltstack02 1  TPASS  :  stgaltstack() fails, Invalid Flag value,errno:22
> > sigaltstack02 2  TFAIL  :  sigaltstack() returned 0, expected -1,errno:12
> 
> I'm still unable to reproduce this failure. Is this with defconfig?
> 
> > Reported Issue in Glibc Bugzilla:
> > Bugfix in Glibc-2.22: [Bug 16850]
> > https://sourceware.org/bugzilla/show_bug.cgi?id=16850
> > 
> > Signed-off-by: Akhilesh Kumar <akhilesh.k@samsung.com>
> > Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
> > Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
> > ---
> > v1 -> Changes in uapi overall header
> > v2 -> Changes done in arm64 headers
> > v3 -> Changes done in both uapi & arm64 headers
> > 
> >  arch/arm64/include/uapi/asm/signal.h |    3 +++
> >  include/uapi/asm-generic/signal.h    |    2 ++
> >  2 files changed, 5 insertions(+)
> 
> Acked-by: Will Deacon <will.deacon@arm.com>
> 
> Arnd: are you planning to take this via asm-generic, or shall I queue it
> on the arm64 fixes branch?
> 

i just noticed this and wanted to note that an old
glibc can fail on a new kernel with this patch if
an application uses MINSIGSTKSZ altstack.

(the specific LTP bug could have been fixed on the libc
side, checking against whatever the libc MINSIGSTKSZ is
as long as it is larger than the kernel value, which
makes sense as userspace may have additional overhead
the kernel is not aware of so please don't try to keep
this in sync with glibc.. hopefully there will not be
another change though).

> Will
> 
> > diff --git a/arch/arm64/include/uapi/asm/signal.h b/arch/arm64/include/uapi/asm/signal.h
> > index 8d1e723..991bf5d 100644
> > --- a/arch/arm64/include/uapi/asm/signal.h
> > +++ b/arch/arm64/include/uapi/asm/signal.h
> > @@ -19,6 +19,9 @@
> >  /* Required for AArch32 compatibility. */
> >  #define SA_RESTORER	0x04000000
> >  
> > +#define MINSIGSTKSZ 5120
> > +#define SIGSTKSZ    16384
> > +
> >  #include <asm-generic/signal.h>
> >  
> >  #endif
> > diff --git a/include/uapi/asm-generic/signal.h b/include/uapi/asm-generic/signal.h
> > index 9df61f1..b98b67b 100644
> > --- a/include/uapi/asm-generic/signal.h
> > +++ b/include/uapi/asm-generic/signal.h
> > @@ -80,8 +80,10 @@
> >   *	SA_RESTORER	0x04000000
> >   */
> >  
> > +#if !defined MINSIGSTKSZ || !defined SIGSTKSZ
> >  #define MINSIGSTKSZ	2048
> >  #define SIGSTKSZ	8192
> > +#endif
> >  
> >  #ifndef __ASSEMBLY__
> >  typedef struct {
> > -- 
> > 1.7.9.5
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Arnd Bergmann]

On Thursday 15 October 2015 14:12:41 Szabolcs Nagy wrote:
> * Will Deacon <will.deacon@arm.com> [2015-10-09 11:33:52 +0100]:
> 
> > On Fri, Oct 09, 2015 at 03:59:40PM +0530, Manjeet Pawar wrote:
> > > MINSIGSTKSZ and SIGSTKSZ for ARM64 are not correctly set in latest kernel.
> > > This patch fixes this issue.
> > > 
> > > This issue is reported in LTP (testcase: sigaltstack02.c).
> > > Testcase failed when sigaltstack() called with stack size "MINSIGSTKSZ - 1"
> > > Since in Glibc-2.22, MINSIGSTKSZ is set to 5120 but in kernel
> > > it is set to 2048 so testcase gets failed.
> > > 
> > > Testcase Output:
> > > sigaltstack02 1  TPASS  :  stgaltstack() fails, Invalid Flag value,errno:22
> > > sigaltstack02 2  TFAIL  :  sigaltstack() returned 0, expected -1,errno:12
> > 
> > I'm still unable to reproduce this failure. Is this with defconfig?
> > 
> > > Reported Issue in Glibc Bugzilla:
> > > Bugfix in Glibc-2.22: [Bug 16850]
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=16850
> > > 
> > > Signed-off-by: Akhilesh Kumar <akhilesh.k@samsung.com>
> > > Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
> > > Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
> > > ---
> > > v1 -> Changes in uapi overall header
> > > v2 -> Changes done in arm64 headers
> > > v3 -> Changes done in both uapi & arm64 headers
> > > 
> > >  arch/arm64/include/uapi/asm/signal.h |    3 +++
> > >  include/uapi/asm-generic/signal.h    |    2 ++
> > >  2 files changed, 5 insertions(+)
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > Arnd: are you planning to take this via asm-generic, or shall I queue it
> > on the arm64 fixes branch?
> > 
> 
> i just noticed this and wanted to note that an old
> glibc can fail on a new kernel with this patch if
> an application uses MINSIGSTKSZ altstack.

Well worth noting, but I think we should consider this intentional:

If an application built against an old glibc uses MINSIGSTKSZ, it
current gets random data corruption of a segmentation fault that
are both hard to track down, while a new kernel would result in
more sensible error code that is easier to debug.

	Arnd

Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Will Deacon]

On Thu, Oct 15, 2015 at 02:12:41PM +0200, Szabolcs Nagy wrote:
> * Will Deacon <will.deacon@arm.com> [2015-10-09 11:33:52 +0100]:
> 
> > On Fri, Oct 09, 2015 at 03:59:40PM +0530, Manjeet Pawar wrote:
> > > MINSIGSTKSZ and SIGSTKSZ for ARM64 are not correctly set in latest kernel.
> > > This patch fixes this issue.
> > > 
> > > This issue is reported in LTP (testcase: sigaltstack02.c).
> > > Testcase failed when sigaltstack() called with stack size "MINSIGSTKSZ - 1"
> > > Since in Glibc-2.22, MINSIGSTKSZ is set to 5120 but in kernel
> > > it is set to 2048 so testcase gets failed.
> > > 
> > > Testcase Output:
> > > sigaltstack02 1  TPASS  :  stgaltstack() fails, Invalid Flag value,errno:22
> > > sigaltstack02 2  TFAIL  :  sigaltstack() returned 0, expected -1,errno:12
> > 
> > I'm still unable to reproduce this failure. Is this with defconfig?
> > 
> > > Reported Issue in Glibc Bugzilla:
> > > Bugfix in Glibc-2.22: [Bug 16850]
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=16850
> > > 
> > > Signed-off-by: Akhilesh Kumar <akhilesh.k@samsung.com>
> > > Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
> > > Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
> > > ---
> > > v1 -> Changes in uapi overall header
> > > v2 -> Changes done in arm64 headers
> > > v3 -> Changes done in both uapi & arm64 headers
> > > 
> > >  arch/arm64/include/uapi/asm/signal.h |    3 +++
> > >  include/uapi/asm-generic/signal.h    |    2 ++
> > >  2 files changed, 5 insertions(+)
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > Arnd: are you planning to take this via asm-generic, or shall I queue it
> > on the arm64 fixes branch?
> > 
> 
> i just noticed this and wanted to note that an old
> glibc can fail on a new kernel with this patch if
> an application uses MINSIGSTKSZ altstack.

I was worried about that (which was also why I didn't Cc stable on this,
because we shouldn't pretend that glibc and the kernel are synchronised),
but the value *is* incorrect and glibc has already been updated. In
other words, not merging this patch is also problematic, because we're
advertising a size that's significantly too small.

Would you rather we kept the old incorrect value in the kernel headers?

Will

Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

[Szabolcs Nagy]

* Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> [2015-10-15 13:47:56 +0100]:
> On Thu, Oct 15, 2015 at 02:12:41PM +0200, Szabolcs Nagy wrote:
> > * Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> [2015-10-09 11:33:52 +0100]:
> > 
> > > On Fri, Oct 09, 2015 at 03:59:40PM +0530, Manjeet Pawar wrote:
> > > > MINSIGSTKSZ and SIGSTKSZ for ARM64 are not correctly set in latest kernel.
> > > > This patch fixes this issue.
> > > > 
> > > > This issue is reported in LTP (testcase: sigaltstack02.c).
> > > > Testcase failed when sigaltstack() called with stack size "MINSIGSTKSZ - 1"
> > > > Since in Glibc-2.22, MINSIGSTKSZ is set to 5120 but in kernel
> > > > it is set to 2048 so testcase gets failed.
> > > > 
> > > > Testcase Output:
> > > > sigaltstack02 1  TPASS  :  stgaltstack() fails, Invalid Flag value,errno:22
> > > > sigaltstack02 2  TFAIL  :  sigaltstack() returned 0, expected -1,errno:12
> > > 
> > > I'm still unable to reproduce this failure. Is this with defconfig?
> > > 
> > > > Reported Issue in Glibc Bugzilla:
> > > > Bugfix in Glibc-2.22: [Bug 16850]
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=16850
> > > > 
> > > > Signed-off-by: Akhilesh Kumar <akhilesh.k-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > Signed-off-by: Manjeet Pawar <manjeet.p-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > Signed-off-by: Rohit Thapliyal <r.thapliyal-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > ---
> > > > v1 -> Changes in uapi overall header
> > > > v2 -> Changes done in arm64 headers
> > > > v3 -> Changes done in both uapi & arm64 headers
> > > > 
> > > >  arch/arm64/include/uapi/asm/signal.h |    3 +++
> > > >  include/uapi/asm-generic/signal.h    |    2 ++
> > > >  2 files changed, 5 insertions(+)
> > > 
> > > Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> > > 
> > > Arnd: are you planning to take this via asm-generic, or shall I queue it
> > > on the arm64 fixes branch?
> > > 
> > 
> > i just noticed this and wanted to note that an old
> > glibc can fail on a new kernel with this patch if
> > an application uses MINSIGSTKSZ altstack.
> 
> I was worried about that (which was also why I didn't Cc stable on this,
> because we shouldn't pretend that glibc and the kernel are synchronised),
> but the value *is* incorrect and glibc has already been updated. In
> other words, not merging this patch is also problematic, because we're
> advertising a size that's significantly too small.
> 
> Would you rather we kept the old incorrect value in the kernel headers?
> 

i think the change is ok and it is good that it is not yet in stable.

(for posix conformance only the value in libc matters though, the
kernel definition is not visible as glibc does not use it and even
in posix the semantics of this macro is not entirely clear so leaving
the wrong value in kernel uapi is not that terrible.. e.g. the x86
definition was not updated when avx extension was added, for abi
stability i guess.  in musl libc this was set to 6K when i did the
aarch64 port, had i used 5000 as the limit i would be upset by such
a change, but i have nothing to complain about now..)