Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-15 Thread Szabolcs Nagy
* Will Deacon  [2015-10-15 13:47:56 +0100]:
> On Thu, Oct 15, 2015 at 02:12:41PM +0200, Szabolcs Nagy wrote:
> > * Will Deacon  [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 
> > > > Signed-off-by: Manjeet Pawar 
> > > > Signed-off-by: Rohit Thapliyal 
> > > > ---
> > > > 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 
> > > 
> > > 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..)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-15 Thread Will Deacon
On Thu, Oct 15, 2015 at 02:12:41PM +0200, Szabolcs Nagy wrote:
> * Will Deacon  [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 
> > > Signed-off-by: Manjeet Pawar 
> > > Signed-off-by: Rohit Thapliyal 
> > > ---
> > > 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 
> > 
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-15 Thread Arnd Bergmann
On Thursday 15 October 2015 14:12:41 Szabolcs Nagy wrote:
> * Will Deacon  [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 
> > > Signed-off-by: Manjeet Pawar 
> > > Signed-off-by: Rohit Thapliyal 
> > > ---
> > > 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 
> > 
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-15 Thread Szabolcs Nagy
* Will Deacon  [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 
> > Signed-off-by: Manjeet Pawar 
> > Signed-off-by: Rohit Thapliyal 
> > ---
> > 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 
> 
> 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_RESTORER0x0400
> >  
> > +#define MINSIGSTKSZ 5120
> > +#define SIGSTKSZ16384
> > +
> >  #include 
> >  
> >  #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 0x0400
> >   */
> >  
> > +#if !defined MINSIGSTKSZ || !defined SIGSTKSZ
> >  #define MINSIGSTKSZ2048
> >  #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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-09 Thread 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

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

we used LTP testsuite (version : - 20140115) , 
compile LTP testsuite with arm64 toolchain (changes backported form Latest 
glibc 2.22)
Then this issue occur.




   
  
 


Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-09 Thread Arnd Bergmann
On Friday 09 October 2015 11:33:52 Will Deacon wrote:

> Acked-by: Will Deacon 
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-09 Thread 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 
> Signed-off-by: Manjeet Pawar 
> Signed-off-by: Rohit Thapliyal 
> ---
> 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 

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  0x0400
>  
> +#define MINSIGSTKSZ 5120
> +#define SIGSTKSZ16384
> +
>  #include 
>  
>  #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 0x0400
>   */
>  
> +#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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCHv3] ARM64:Fix MINSIGSTKSZ and SIGSTKSZ

2015-10-09 Thread 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 
Signed-off-by: Manjeet Pawar 
Signed-off-by: Rohit Thapliyal 
---
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_RESTORER0x0400
 
+#define MINSIGSTKSZ 5120
+#define SIGSTKSZ16384
+
 #include 
 
 #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 0x0400
  */
 
+#if !defined MINSIGSTKSZ || !defined SIGSTKSZ
 #define MINSIGSTKSZ2048
 #define SIGSTKSZ   8192
+#endif
 
 #ifndef __ASSEMBLY__
 typedef struct {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/