Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Adam Borowski]

On Wed, 6 Apr 2016, Geert Uytterhoeven wrote:
> On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
>>  v6:
>>  - time_t, __kenel_off_t and other types turned to be 32-bit
>>    for compatibility reasons (after v5 discussion);

Introducing a new arch today with y2038 problems is not a good idea.
Linus said so with appropriately pointy words in 2011.

> What makes you think these "applications that can’t readily be migrated to LP64
> because they were written assuming an ILP32 data model, and that will never
> become suitable for a LP64 data model and will remain locked into ILP32
> operating environments" are more likely to be fixed for y2038 later, than for
> LP64 now?

Such broken applications already have plenty of bogus architecture detection
code so you need porting anyway...

> We're already closer to the (future) y2038 than to the (past) introduction of
> LP64...
>
> These unfixable legacy applications have been spreading through x32 to
> the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
> or is it planned)? Lots of resources are spent on maintaining the status quo,
> instead of on fixing the real problems.
          
As an x32 (userland) porter, I can tell you that time_t!=long _did_ cause
non-trivial amounts of work.  But that work is already done (at least in
Debian), so you might as well benefit from it.


-- 
A tit a day keeps the vet away.

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Adam Borowski]

On Wed, 6 Apr 2016, Geert Uytterhoeven wrote:
> On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
>>  v6:
>>  - time_t, __kenel_off_t and other types turned to be 32-bit
>>    for compatibility reasons (after v5 discussion);

Introducing a new arch today with y2038 problems is not a good idea.
Linus said so with appropriately pointy words in 2011.

> What makes you think these "applications that can’t readily be migrated to LP64
> because they were written assuming an ILP32 data model, and that will never
> become suitable for a LP64 data model and will remain locked into ILP32
> operating environments" are more likely to be fixed for y2038 later, than for
> LP64 now?

Such broken applications already have plenty of bogus architecture detection
code so you need porting anyway...

> We're already closer to the (future) y2038 than to the (past) introduction of
> LP64...
>
> These unfixable legacy applications have been spreading through x32 to
> the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
> or is it planned)? Lots of resources are spent on maintaining the status quo,
> instead of on fixing the real problems.
          
As an x32 (userland) porter, I can tell you that time_t!=long _did_ cause
non-trivial amounts of work.  But that work is already done (at least in
Debian), so you might as well benefit from it.


-- 
A tit a day keeps the vet away.

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Andrew Pinski]

On Thu, Apr 7, 2016 at 5:18 AM, Adam Borowski <kilobyte@angband.pl> wrote:
> On Wed, 6 Apr 2016, Geert Uytterhoeven wrote:
>> On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
>>>  v6:
>>>  - time_t, __kenel_off_t and other types turned to be 32-bit
>>>    for compatibility reasons (after v5 discussion);
>
> Introducing a new arch today with y2038 problems is not a good idea.
> Linus said so with appropriately pointy words in 2011.

This is the third time we had this discussion on time_t for ILP32.  I
had originally it as 32bit, then Catalin suggested I change it to
64bit and then Arnd (with his work for 2038 issue on 32bit arch) said
ILP32 should match all other 32bit targets and the other 64bit time_t
be fixed by the current work he was working on.  Now you are
suggesting we change it again.
Arnd can you please comment more on why we want 32bit time_t instead
of the 64bit one?  I Know there was some POSIX (or was it C90)
violation but I suspect there is an easy way to workaround this inside
the kernel but the discussion to move over to 32bit time_t was already
made by the time I started to look into that.

>
>> What makes you think these "applications that can’t readily be migrated to LP64
>> because they were written assuming an ILP32 data model, and that will never
>> become suitable for a LP64 data model and will remain locked into ILP32
>> operating environments" are more likely to be fixed for y2038 later, than for
>> LP64 now?
>
> Such broken applications already have plenty of bogus architecture detection
> code so you need porting anyway...

I don't disagree here; just see below ...

>
>> We're already closer to the (future) y2038 than to the (past) introduction of
>> LP64...
>>
>> These unfixable legacy applications have been spreading through x32 to
>> the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
>> or is it planned)? Lots of resources are spent on maintaining the status quo,
>> instead of on fixing the real problems.
>
> As an x32 (userland) porter, I can tell you that time_t!=long _did_ cause
> non-trivial amounts of work.  But that work is already done (at least in
> Debian), so you might as well benefit from it.

There is actually private code out there which uses timespec and
timeval to pass time over the wire; yes I know bad coding style and
all but they did it that way.  This is code which was working for x86
and we are porting it to ARM64; a data center code by the way; not
some networking code even.  This means they have not ported the code
to fully 64bit yet and they might never.

Thanks,
Andrew

>
>
> --
> A tit a day keeps the vet away.

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Arnd Bergmann]

On Friday 08 April 2016, Andrew Pinski wrote:
> On Thu, Apr 7, 2016 at 5:18 AM, Adam Borowski <kilobyte@angband.pl> wrote:
> > On Wed, 6 Apr 2016, Geert Uytterhoeven wrote:
> >> On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> >>>  v6:
> >>>  - time_t, __kenel_off_t and other types turned to be 32-bit
> >>>    for compatibility reasons (after v5 discussion);
> >
> > Introducing a new arch today with y2038 problems is not a good idea.
> > Linus said so with appropriately pointy words in 2011.

This was before we made the decision to fix the y2038 problem for all
architectures.

> This is the third time we had this discussion on time_t for ILP32.  I
> had originally it as 32bit, then Catalin suggested I change it to
> 64bit and then Arnd (with his work for 2038 issue on 32bit arch) said
> ILP32 should match all other 32bit targets and the other 64bit time_t
> be fixed by the current work he was working on.  Now you are
> suggesting we change it again.
> Arnd can you please comment more on why we want 32bit time_t instead
> of the 64bit one?  I Know there was some POSIX (or was it C90)
> violation but I suspect there is an easy way to workaround this inside
> the kernel but the discussion to move over to 32bit time_t was already
> made by the time I started to look into that.

x32 still runs into new problems today, and will continue to have problems
with newly added drivers that pass time_t (or other __kernel_long_t) arguments
through ioctl.

To avoid having to audit every new driver for interfaces that behave
differently based on the __kernel_long_t definition, arm64 is not following
the same route as x86 here and instead uses the normal 32-bit ABI like
any other architecture. This means we use 32-bit time_t, aio_context_t,
size_t and clock_t and share the system call implementation with the
compat handling for arm (aarch32) mode.

Once we have the interfaces for 64-bit time_t in place in the kernel,
we will be able to rebuild glibc on all 32-bit architectures including
arm and arm64/ilp32 that way.

The POSIX and C99 incompatibility you mention is about struct timespec,
which uses 'long' as the type for the tv_nsec member. This is vaguely
related to the issue of 64-bit time_t, but is not the reason for
starting out with 32-bit time_t for the new ABI here.

[side note:
How to precisely handle tv_nsec on 32-bit architectures is still an open
issue that will have to be solved when we nail down the new system call
interfaces:
The issue specifically is what happens when the upper half of the
second 64-bit word in struct timespec argument passed into a system
call is nonzero: the normal 64-bit syscalls must return an error,
while the 32-bit user space expects the kernel to ignore the upper bits.
This means something between the application and the native system call
has to clear the bits, and this can either be done by copying the
data inside of glibc (as done on x32) or by adding an extra system
call entry point in the kernel.]

> >> We're already closer to the (future) y2038 than to the (past) introduction of
> >> LP64...
> >>
> >> These unfixable legacy applications have been spreading through x32 to
> >> the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
> >> or is it planned)? Lots of resources are spent on maintaining the status quo,
> >> instead of on fixing the real problems.
> >
> > As an x32 (userland) porter, I can tell you that time_t!=long _did_ cause
> > non-trivial amounts of work.  But that work is already done (at least in
> > Debian), so you might as well benefit from it.
> 
> There is actually private code out there which uses timespec and
> timeval to pass time over the wire; yes I know bad coding style and
> all but they did it that way.  This is code which was working for x86
> and we are porting it to ARM64; a data center code by the way; not
> some networking code even.  This means they have not ported the code
> to fully 64bit yet and they might never.

This code will run into the same problem on arm64/ilp32 when built against
a future libc implementation that defines time_t as 64-bit, but at least
the glibc maintainers so far plan to leave this as a per-application
option for the forseeable future: even on a system that uses 64-bit time_t
in user space and kernel by default, you should be able to build an
application using a 32-bit time_t.

	Arnd

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Arnd Bergmann]

On Friday 08 April 2016, Andrew Pinski wrote:
> On Thu, Apr 7, 2016 at 5:18 AM, Adam Borowski <kilobyte@angband.pl> wrote:
> > On Wed, 6 Apr 2016, Geert Uytterhoeven wrote:
> >> On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> >>>  v6:
> >>>  - time_t, __kenel_off_t and other types turned to be 32-bit
> >>>    for compatibility reasons (after v5 discussion);
> >
> > Introducing a new arch today with y2038 problems is not a good idea.
> > Linus said so with appropriately pointy words in 2011.

This was before we made the decision to fix the y2038 problem for all
architectures.

> This is the third time we had this discussion on time_t for ILP32.  I
> had originally it as 32bit, then Catalin suggested I change it to
> 64bit and then Arnd (with his work for 2038 issue on 32bit arch) said
> ILP32 should match all other 32bit targets and the other 64bit time_t
> be fixed by the current work he was working on.  Now you are
> suggesting we change it again.
> Arnd can you please comment more on why we want 32bit time_t instead
> of the 64bit one?  I Know there was some POSIX (or was it C90)
> violation but I suspect there is an easy way to workaround this inside
> the kernel but the discussion to move over to 32bit time_t was already
> made by the time I started to look into that.

x32 still runs into new problems today, and will continue to have problems
with newly added drivers that pass time_t (or other __kernel_long_t) arguments
through ioctl.

To avoid having to audit every new driver for interfaces that behave
differently based on the __kernel_long_t definition, arm64 is not following
the same route as x86 here and instead uses the normal 32-bit ABI like
any other architecture. This means we use 32-bit time_t, aio_context_t,
size_t and clock_t and share the system call implementation with the
compat handling for arm (aarch32) mode.

Once we have the interfaces for 64-bit time_t in place in the kernel,
we will be able to rebuild glibc on all 32-bit architectures including
arm and arm64/ilp32 that way.

The POSIX and C99 incompatibility you mention is about struct timespec,
which uses 'long' as the type for the tv_nsec member. This is vaguely
related to the issue of 64-bit time_t, but is not the reason for
starting out with 32-bit time_t for the new ABI here.

[side note:
How to precisely handle tv_nsec on 32-bit architectures is still an open
issue that will have to be solved when we nail down the new system call
interfaces:
The issue specifically is what happens when the upper half of the
second 64-bit word in struct timespec argument passed into a system
call is nonzero: the normal 64-bit syscalls must return an error,
while the 32-bit user space expects the kernel to ignore the upper bits.
This means something between the application and the native system call
has to clear the bits, and this can either be done by copying the
data inside of glibc (as done on x32) or by adding an extra system
call entry point in the kernel.]

> >> We're already closer to the (future) y2038 than to the (past) introduction of
> >> LP64...
> >>
> >> These unfixable legacy applications have been spreading through x32 to
> >> the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
> >> or is it planned)? Lots of resources are spent on maintaining the status quo,
> >> instead of on fixing the real problems.
> >
> > As an x32 (userland) porter, I can tell you that time_t!=long _did_ cause
> > non-trivial amounts of work.  But that work is already done (at least in
> > Debian), so you might as well benefit from it.
> 
> There is actually private code out there which uses timespec and
> timeval to pass time over the wire; yes I know bad coding style and
> all but they did it that way.  This is code which was working for x86
> and we are porting it to ARM64; a data center code by the way; not
> some networking code even.  This means they have not ported the code
> to fully 64bit yet and they might never.

This code will run into the same problem on arm64/ilp32 when built against
a future libc implementation that defines time_t as 64-bit, but at least
the glibc maintainers so far plan to leave this as a per-application
option for the forseeable future: even on a system that uses 64-bit time_t
in user space and kernel by default, you should be able to build an
application using a 32-bit time_t.

	Arnd

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Arnd Bergmann]

On Tuesday 17 May 2016 18:02:36 Andreas Schwab wrote:
> Joseph Myers <joseph@codesourcery.com> writes:
> 
> > On Tue, 17 May 2016, Arnd Bergmann wrote:
> >
> >> I think it has become easier to override now and we just need to
> >> update sysdeps/unix/sysv/linux/generic/bits/typesizes.h to set
> >> 
> >> #define __INO64_T_TYPE          __UQUAD_TYPE
> >> #define __OFF64_T_TYPE          __UQUAD_TYPE
> >> #define __OFF_T_MATCHES_OFF64_T        1
> >> #define __INO_T_MATCHES_INO64_T        1
> >> 
> >> for new architectures (obviously not the ones that already use the
> >> 32-bit types). I haven't tries this, so there may be other things
> >> that are required.
> >
> > I think more than that would be needed to get struct stat to match and get 
> > things aliased for that (which is presumably desirable).
> 
> Looking at sysdeps/unix/sysv/linux/generic/bits/stat.h, there is at
> least blkcnt_t that differs between stat and stat64.  And you probably
> want to alias statfs and statfs64 as well, ie. fs{blk,fil}cnt_t.

Makes sense. Indeed that is a bit more work than I hoped, in particular
if we have to audit the uses of __WORDSIZE as well.

	Arnd

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Arnd Bergmann]

On Tuesday 17 May 2016 18:02:36 Andreas Schwab wrote:
> Joseph Myers <joseph@codesourcery.com> writes:
> 
> > On Tue, 17 May 2016, Arnd Bergmann wrote:
> >
> >> I think it has become easier to override now and we just need to
> >> update sysdeps/unix/sysv/linux/generic/bits/typesizes.h to set
> >> 
> >> #define __INO64_T_TYPE          __UQUAD_TYPE
> >> #define __OFF64_T_TYPE          __UQUAD_TYPE
> >> #define __OFF_T_MATCHES_OFF64_T        1
> >> #define __INO_T_MATCHES_INO64_T        1
> >> 
> >> for new architectures (obviously not the ones that already use the
> >> 32-bit types). I haven't tries this, so there may be other things
> >> that are required.
> >
> > I think more than that would be needed to get struct stat to match and get 
> > things aliased for that (which is presumably desirable).
> 
> Looking at sysdeps/unix/sysv/linux/generic/bits/stat.h, there is at
> least blkcnt_t that differs between stat and stat64.  And you probably
> want to alias statfs and statfs64 as well, ie. fs{blk,fil}cnt_t.

Makes sense. Indeed that is a bit more work than I hoped, in particular
if we have to audit the uses of __WORDSIZE as well.

	Arnd

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Andreas Schwab]

Joseph Myers <joseph@codesourcery.com> writes:

> On Tue, 17 May 2016, Arnd Bergmann wrote:
>
>> I think it has become easier to override now and we just need to
>> update sysdeps/unix/sysv/linux/generic/bits/typesizes.h to set
>> 
>> #define __INO64_T_TYPE          __UQUAD_TYPE
>> #define __OFF64_T_TYPE          __UQUAD_TYPE
>> #define __OFF_T_MATCHES_OFF64_T        1
>> #define __INO_T_MATCHES_INO64_T        1
>> 
>> for new architectures (obviously not the ones that already use the
>> 32-bit types). I haven't tries this, so there may be other things
>> that are required.
>
> I think more than that would be needed to get struct stat to match and get 
> things aliased for that (which is presumably desirable).

Looking at sysdeps/unix/sysv/linux/generic/bits/stat.h, there is at
least blkcnt_t that differs between stat and stat64.  And you probably
want to alias statfs and statfs64 as well, ie. fs{blk,fil}cnt_t.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Andreas Schwab]

Joseph Myers <joseph@codesourcery.com> writes:

> On Tue, 17 May 2016, Arnd Bergmann wrote:
>
>> I think it has become easier to override now and we just need to
>> update sysdeps/unix/sysv/linux/generic/bits/typesizes.h to set
>> 
>> #define __INO64_T_TYPE          __UQUAD_TYPE
>> #define __OFF64_T_TYPE          __UQUAD_TYPE
>> #define __OFF_T_MATCHES_OFF64_T        1
>> #define __INO_T_MATCHES_INO64_T        1
>> 
>> for new architectures (obviously not the ones that already use the
>> 32-bit types). I haven't tries this, so there may be other things
>> that are required.
>
> I think more than that would be needed to get struct stat to match and get 
> things aliased for that (which is presumably desirable).

Looking at sysdeps/unix/sysv/linux/generic/bits/stat.h, there is at
least blkcnt_t that differs between stat and stat64.  And you probably
want to alias statfs and statfs64 as well, ie. fs{blk,fil}cnt_t.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Joseph Myers]

On Tue, 17 May 2016, Arnd Bergmann wrote:

> I think it has become easier to override now and we just need to
> update sysdeps/unix/sysv/linux/generic/bits/typesizes.h to set
> 
> #define __INO64_T_TYPE          __UQUAD_TYPE
> #define __OFF64_T_TYPE          __UQUAD_TYPE
> #define __OFF_T_MATCHES_OFF64_T        1
> #define __INO_T_MATCHES_INO64_T        1
> 
> for new architectures (obviously not the ones that already use the
> 32-bit types). I haven't tries this, so there may be other things
> that are required.

I think more than that would be needed to get struct stat to match and get 
things aliased for that (which is presumably desirable).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Joseph Myers]

On Tue, 17 May 2016, Szabolcs Nagy wrote:

> i think even legacy software should be able to deal with 64bit off_t,
> so we could avoid having two sets of filesystem apis or is 64bit-only
> off_t more work to do in linux/glibc?

wordsize-64 directories generally expect 64-bit interfaces.  wordsize-32 
directories generally expect that there are two sets of filesystem APIs 
which are not aliased (at the userspace level - the versions for the 
generic syscall API deal with setting EOVERFLOW in userspace as needed).

The "wordsize" concept is not wonderfully well-defined and could do with 
being split up into multiple better-defined concepts, but that's obvious 
something pretty tricky to get right, involving a very careful analysis of 
the existing code.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Joseph Myers]

On Tue, 17 May 2016, Szabolcs Nagy wrote:

> i think even legacy software should be able to deal with 64bit off_t,
> so we could avoid having two sets of filesystem apis or is 64bit-only
> off_t more work to do in linux/glibc?

wordsize-64 directories generally expect 64-bit interfaces.  wordsize-32 
directories generally expect that there are two sets of filesystem APIs 
which are not aliased (at the userspace level - the versions for the 
generic syscall API deal with setting EOVERFLOW in userspace as needed).

The "wordsize" concept is not wonderfully well-defined and could do with 
being split up into multiple better-defined concepts, but that's obvious 
something pretty tricky to get right, involving a very careful analysis of 
the existing code.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Arnd Bergmann]

On Tuesday 17 May 2016 13:10:53 Szabolcs Nagy wrote:
> On 05/04/16 23:08, Yury Norov wrote:
> > This version is rebased on kernel v4.6-rc2, and has fixes in signal subsystem.
> > It works with updated glibc [1] (though very draft), and tested with LTP.
> > 
> > It was tested on QEMU and ThunderX machines. No major difference found.
> > This is RFC because ILP32 is not tested in big-endian mode.
> > 
> >  v3: https://lkml.org/lkml/2014/9/3/704
> >  v4: https://lkml.org/lkml/2015/4/13/691
> >  v5: https://lkml.org/lkml/2015/9/29/911
> > 
> >  v6:
> >  - time_t, __kenel_off_t and other types turned to be 32-bit
> >    for compatibility reasons (after v5 discussion);
> 
> i added libc-alpha to cc, the thread is at
> https://lkml.org/lkml/2016/4/5/872
> or http://thread.gmane.org/gmane.linux.kernel.cross-arch/31449
> 
> the kernel position seems to be that the aarch64 ilp32 syscall abi
> will follow regular 32bit abis (apart from signal context and ptrace
> related things and syscalls will take 64bit arguments).
> 
> i think the reasoning is that this allows legacy (non-64bit-safe)
> software to use this abi on aarch64 without trouble.
> 
> i think this design should be ok for glibc.
> 
> there are some interop issues between processes following different abis
> (e.g. shared mutexes, abi specific file formats, lib paths,..) which apply,
> but i assume it is no different from existing 32bit vs 64bit issues.
> 
> the 32bit time_t and off_t are ugly and i wonder if 32bit __kernel_off_t
> is really necessary. (i don't see where that is changed: it is supposed
> to be __kernel_long_t which is 64bit).

__kernel_off_t and __kernel_ino_t are always 'long' and 'unsigned long'
for historic reasons, but they are not used at all on modern architectures.
We could add a few lines with #ifdef to the kernel headers to hide the
definition on architectures that don't use them, but that seems pointless
given that glibc just defines its own version.

For time_t, I'm working on the same thing in the long run: there will
be a 64 bit __kernel_time64_t that is always used on all architectures,
and a __kernel_time_t that is provided for backwards compatibility
with old libc on architectures that started out having it.

__kernel_long_t should not be 64 bit on 32-bit architectures, it is the
size of the 'long' type in the ABI and defining it as 'long long' on
x32 keeps causing problems that we should not add on new architectures.

> i think even legacy software should be able to deal with 64bit off_t,
> so we could avoid having two sets of filesystem apis or is 64bit-only
> off_t more work to do in linux/glibc?

All architectures that got merged in the last 10 years only support
64-bit __kernel_loff_t in the kernel but not __kernel_off_t, this
includes arc, c6x, h8300, hexagon, metag, nios2, openrisc, tile and
unicore32. However, glibc implements 32-bit off_t on top of the
64-bit syscall interfaces. I believe this was done to keep the code
looking more like the older 32-bit architectures.

I think it has become easier to override now and we just need to
update sysdeps/unix/sysv/linux/generic/bits/typesizes.h to set

#define __INO64_T_TYPE          __UQUAD_TYPE
#define __OFF64_T_TYPE          __UQUAD_TYPE
#define __OFF_T_MATCHES_OFF64_T        1
#define __INO_T_MATCHES_INO64_T        1

for new architectures (obviously not the ones that already use the
32-bit types). I haven't tries this, so there may be other things
that are required.

	Arnd

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Szabolcs Nagy]

On 05/04/16 23:08, Yury Norov wrote:
> This version is rebased on kernel v4.6-rc2, and has fixes in signal subsystem.
> It works with updated glibc [1] (though very draft), and tested with LTP.
> 
> It was tested on QEMU and ThunderX machines. No major difference found.
> This is RFC because ILP32 is not tested in big-endian mode.
> 
>  v3: https://lkml.org/lkml/2014/9/3/704
>  v4: https://lkml.org/lkml/2015/4/13/691
>  v5: https://lkml.org/lkml/2015/9/29/911
> 
>  v6:
>  - time_t, __kenel_off_t and other types turned to be 32-bit
>    for compatibility reasons (after v5 discussion);

i added libc-alpha to cc, the thread is at
https://lkml.org/lkml/2016/4/5/872
or http://thread.gmane.org/gmane.linux.kernel.cross-arch/31449

the kernel position seems to be that the aarch64 ilp32 syscall abi
will follow regular 32bit abis (apart from signal context and ptrace
related things and syscalls will take 64bit arguments).

i think the reasoning is that this allows legacy (non-64bit-safe)
software to use this abi on aarch64 without trouble.

i think this design should be ok for glibc.

there are some interop issues between processes following different abis
(e.g. shared mutexes, abi specific file formats, lib paths,..) which apply,
but i assume it is no different from existing 32bit vs 64bit issues.

the 32bit time_t and off_t are ugly and i wonder if 32bit __kernel_off_t
is really necessary. (i don't see where that is changed: it is supposed
to be __kernel_long_t which is 64bit).

i think even legacy software should be able to deal with 64bit off_t,
so we could avoid having two sets of filesystem apis or is 64bit-only
off_t more work to do in linux/glibc?

>  - related changes applied to ILP32 syscall table and handlers;
>  - ILP32 VDSO code excluded. It's not mandatory, and caused questions
>    during review process. We definitely make sure we will follow up
>    with a VDSO later on because it is needed for performance reasons;
>  - fixed build issues with different combinations of AARCH32 / ILP32
>    enabling in config;
>  - ILP32 TLS bug fixed;
>  - entry32-common.S introduced to hold wrappers needed for both ILP32
>    and AARCH32_EL0;
>  - documentation updated according to latest changes;
>  - rebased to the current head;
>  - coding style re-checked;
>  - ILP32 syscall table turned around.
> 
>    rfc3:
>  - all structures and system calls are just like AARCH32 ones now. with 2
>    exceptions: syscalls that take 64-bit parameter in 2 32-bit regosters
>    are replaced with LP64 version; struct rt_sigframe is constructed both
>    from LP64 and AARCH32 fields to be consistent with AARCH64 register set;
>  - documentation rewritten accordingly;
>  - common code for all 3 ABIs is moved to separated files for easy use,
>    new headers and objects are introduced, incl: is_compat.h, thread_bits.h,
>    signal_common.h, signal32_common.h.
>  - ILP32 VDSO code restored, Nathans comments are addressed;
>  - patch "arm64: ilp32: force IPC_64 in msgctl, shmctl, semctl" removed, as
>    Arnd suggested general solution for IPC_64 problem.
> 
>    rfc4:
>  - sys_ilp32.c syscall list is fixed according to comments;
>  - binfmt_elf32.c and binfmt_ilp32.c are introduced to host the code handling
>    corresponding formats;
>  - statfs64, fstsatfs64 and mmap wrappers are removed;
>  - rebased on v4.4-rc8 + http://www.spinics.net/lists/kernel/msg2151759.html
> 
>  rfc5:
>  - addressed rfc4 comments;
>  - turned s390 compat wrappers to be generic and applied it to arm64/ilp32.
>    Heiko Carsten and Martin Schwidefsky added to CC as s390 maintainers.
> 
>  rfc6:
>  - glibc follows new ABI, [1];
>  - significant rework for signal subsystem (patches 21, 23) - struct ucontext
>    is now corresponds user representation;
>  - compat wrappers and 32-bit off_t patchsets are joined with this patchset,
>    as for now ilp32 is the only user for them;
>  - moved to kernel v4.6-rc2;
>  - few minor bugfixes.
> 
> [1] https://github.com/norov/glibc/tree/new-api
> 
> Andrew Pinski (7):
>   arm64: ensure the kernel is compiled for LP64
>   arm64: rename COMPAT to AARCH32_EL0 in Kconfig
>   arm64: change some CONFIG_COMPAT over to use CONFIG_AARCH32_EL0
>     instead
>   arm64:uapi: set __BITS_PER_LONG correctly for ILP32 and LP64
>   arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use
>     it
>   arm64: ilp32: introduce ilp32-specific handlers for sigframe and
>     ucontext
>   arm64:ilp32: add ARM64_ILP32 to Kconfig
> 
> Bamvor Jian Zhang (1):
>   arm64: compat: change config dependences to aarch32
> 
> Philipp Tomsich (1):
>   arm64:ilp32: add vdso-ilp32 and use for signal return
> 
> Yury Norov (16):
>   all: syscall wrappers: add documentation
>   all: introduce COMPAT_WRAPPER option and enable it for s390
>   all: s390: move wrapper infrastructure to generic headers
>   all: s390: move compat_wrappers.c from arch/s390/kernel to kernel/
>   all: wrap needed syscalls in generic unistd
>   compat ABI: use non-compat openat and open_by_handle_at variants
>   32-bit ABI: introduce ARCH_32BIT_OFF_T config option
>   arm64: ilp32: add documentation on the ILP32 ABI for ARM64
>   thread: move thread bits accessors to separated file
>   arm64: introduce is_a32_task and is_a32_thread (for AArch32 compat)
>   arm64: ilp32: add is_ilp32_compat_{task,thread} and TIF_32BIT_AARCH64
>   arm64: introduce binfmt_elf32.c
>   arm64: ilp32: introduce binfmt_ilp32.c
>   arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32
>   arm64: signal: share lp64 signal routines to ilp32
>   arm64: signal32: move ilp32 and aarch32 common code to separated file
> 
>  Documentation/adding-syscalls.txt             |  32 +++
>  Documentation/arm64/ilp32.txt                 |  13 ++
>  arch/Kconfig                                  |   8 +
>  arch/arc/Kconfig                              |   1 +
>  arch/arm/Kconfig                              |   1 +
>  arch/arm64/Kconfig                            |  17 +-
>  arch/arm64/Makefile                           |   5 +
>  arch/arm64/include/asm/compat.h               |  19 +-
>  arch/arm64/include/asm/elf.h                  |  35 +---
>  arch/arm64/include/asm/fpsimd.h               |   2 +-
>  arch/arm64/include/asm/ftrace.h               |   2 +-
>  arch/arm64/include/asm/hwcap.h                |   6 +-
>  arch/arm64/include/asm/is_compat.h            |  84 ++++++++
>  arch/arm64/include/asm/memory.h               |   3 +-
>  arch/arm64/include/asm/processor.h            |  11 +-
>  arch/arm64/include/asm/ptrace.h               |   2 +-
>  arch/arm64/include/asm/signal32.h             |   6 +-
>  arch/arm64/include/asm/signal32_common.h      |  25 +++
>  arch/arm64/include/asm/signal_common.h        |  33 +++
>  arch/arm64/include/asm/signal_ilp32.h         |  34 ++++
>  arch/arm64/include/asm/syscall.h              |   2 +-
>  arch/arm64/include/asm/thread_info.h          |   3 +-
>  arch/arm64/include/asm/unistd.h               |  11 +-
>  arch/arm64/include/asm/unistd32.h             |   2 +-
>  arch/arm64/include/asm/vdso.h                 |   6 +
>  arch/arm64/include/uapi/asm/bitsperlong.h     |   9 +-
>  arch/arm64/kernel/Makefile                    |  12 +-
>  arch/arm64/kernel/asm-offsets.c               |   2 +-
>  arch/arm64/kernel/binfmt_elf32.c              |  33 +++
>  arch/arm64/kernel/binfmt_ilp32.c              |  91 +++++++++
>  arch/arm64/kernel/cpufeature.c                |   8 +-
>  arch/arm64/kernel/cpuinfo.c                   |   4 +-
>  arch/arm64/kernel/entry.S                     |  18 +-
>  arch/arm64/kernel/entry_ilp32.S               |  23 +++
>  arch/arm64/kernel/head.S                      |   2 +-
>  arch/arm64/kernel/hw_breakpoint.c             |  10 +-
>  arch/arm64/kernel/perf_regs.c                 |   2 +-
>  arch/arm64/kernel/process.c                   |   7 +-
>  arch/arm64/kernel/ptrace.c                    |  67 ++++++-
>  arch/arm64/kernel/signal.c                    | 100 ++++++----
>  arch/arm64/kernel/signal32.c                  |  85 --------
>  arch/arm64/kernel/signal32_common.c           | 115 +++++++++++
>  arch/arm64/kernel/signal_ilp32.c              | 192 ++++++++++++++++++
>  arch/arm64/kernel/sys32.c                     |   1 +
>  arch/arm64/kernel/sys_ilp32.c                 |  68 +++++++
>  arch/arm64/kernel/traps.c                     |   5 +-
>  arch/arm64/kernel/vdso-ilp32/.gitignore       |   2 +
>  arch/arm64/kernel/vdso-ilp32/Makefile         |  72 +++++++
>  arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S     |  33 +++
>  arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S |  95 +++++++++
>  arch/arm64/kernel/vdso.c                      |  65 ++++--
>  arch/blackfin/Kconfig                         |   1 +
>  arch/cris/Kconfig                             |   1 +
>  arch/frv/Kconfig                              |   1 +
>  arch/h8300/Kconfig                            |   1 +
>  arch/hexagon/Kconfig                          |   1 +
>  arch/m32r/Kconfig                             |   1 +
>  arch/m68k/Kconfig                             |   1 +
>  arch/metag/Kconfig                            |   1 +
>  arch/microblaze/Kconfig                       |   1 +
>  arch/mips/Kconfig                             |   1 +
>  arch/mn10300/Kconfig                          |   1 +
>  arch/nios2/Kconfig                            |   1 +
>  arch/openrisc/Kconfig                         |   1 +
>  arch/parisc/Kconfig                           |   1 +
>  arch/powerpc/Kconfig                          |   1 +
>  arch/s390/Kconfig                             |   1 +
>  arch/s390/include/asm/compat.h                |  17 +-
>  arch/s390/kernel/Makefile                     |   2 +-
>  arch/s390/kernel/compat_linux.c               |   4 +
>  arch/s390/kernel/compat_wrapper.c             | 180 -----------------
>  arch/score/Kconfig                            |   1 +
>  arch/sh/Kconfig                               |   1 +
>  arch/sparc/Kconfig                            |   1 +
>  arch/tile/Kconfig                             |   1 +
>  arch/tile/kernel/compat.c                     |   3 +
>  arch/unicore32/Kconfig                        |   1 +
>  arch/x86/Kconfig                              |   1 +
>  arch/x86/um/Kconfig                           |   1 +
>  arch/xtensa/Kconfig                           |   1 +
>  drivers/clocksource/arm_arch_timer.c          |   2 +-
>  include/linux/compat.h                        | 277 ++++++++++++++++++++++++++
>  include/linux/fcntl.h                         |   2 +-
>  include/linux/ptrace.h                        |   6 +
>  include/linux/syscalls.h                      |  57 +-----
>  include/linux/syscalls_structs.h              |  60 ++++++
>  include/linux/thread_bits.h                   |  55 +++++
>  include/linux/thread_info.h                   |  44 +---
>  include/uapi/asm-generic/unistd.h             | 231 ++++++++++-----------
>  kernel/Makefile                               |   1 +
>  kernel/compat_wrapper.c                       | 175 ++++++++++++++++
>  kernel/ptrace.c                               |  10 +-
>  92 files changed, 1997 insertions(+), 637 deletions(-)
>  create mode 100644 Documentation/arm64/ilp32.txt
>  create mode 100644 arch/arm64/include/asm/is_compat.h
>  create mode 100644 arch/arm64/include/asm/signal32_common.h
>  create mode 100644 arch/arm64/include/asm/signal_common.h
>  create mode 100644 arch/arm64/include/asm/signal_ilp32.h
>  create mode 100644 arch/arm64/kernel/binfmt_elf32.c
>  create mode 100644 arch/arm64/kernel/binfmt_ilp32.c
>  create mode 100644 arch/arm64/kernel/entry_ilp32.S
>  create mode 100644 arch/arm64/kernel/signal32_common.c
>  create mode 100644 arch/arm64/kernel/signal_ilp32.c
>  create mode 100644 arch/arm64/kernel/sys_ilp32.c
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
>  delete mode 100644 arch/s390/kernel/compat_wrapper.c
>  create mode 100644 include/linux/syscalls_structs.h
>  create mode 100644 include/linux/thread_bits.h
>  create mode 100644 kernel/compat_wrapper.c
> 

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 09:28:03AM +0000, Catalin Marinas wrote:
> > >>>>>On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > >>>>>>I debugged preadv02 and pwritev02 failures and found very weird bug.
> > >>>>>>Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > >>>>>>of vector, and kernel reports successful read/write.
[...]
> The discussion is mainly around whether USER_DS for 32-bit compat apps
> should be the same as USER_DS for native 32-bit apps. Even for native
> 32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
> 0xffffffff would fail in both cases anyway. I think the LTP test doesn't
> even try to access such memory but only to probe the range validity (I
> haven't managed to build the latest LTP yet).

OK, so I managed to get the latest LTP (tag 20160510) built for AArch32
but the preadv02 does not fail:

-----------------------------
# uname -m
aarch64

# file ./testcases/bin/preadv02
./testcases/bin/preadv02: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 2.6.32, BuildID[sha1]=98066291426e1d3ee49d8504ce2a5bd721ab7fe8, not stripped

# ./testcases/bin/preadv02
preadv02.c:97: PASS: preadv() failed as expected: EINVAL
preadv02.c:97: PASS: preadv() failed as expected: EINVAL
preadv02.c:97: PASS: preadv() failed as expected: EINVAL
preadv02.c:97: PASS: preadv() failed as expected: EFAULT
preadv02.c:97: PASS: preadv() failed as expected: EBADF
preadv02.c:97: PASS: preadv() failed as expected: EBADF
preadv02.c:97: PASS: preadv() failed as expected: EISDIR
preadv02.c:97: PASS: preadv() failed as expected: ESPIPE

Summary:
passed   8
failed   0
skipped  0
warnings 0
-----------------------------

It's the 4th test above which passes iovec_base as -1 and expects
EFAULT. However, it seems to get the expected error. I guess that even
if access_ok() passes, the access to that location by the kernel would
fail since there isn't anything mapped.

With ILP32, however, STACK_TOP is TASK_SIZE and you may have the address
mapped already. I still think the USER_DS fix is useful, though I need
another way to check that it's actually a fix.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 09:28:03AM +0000, Catalin Marinas wrote:
> > >>>>>On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > >>>>>>I debugged preadv02 and pwritev02 failures and found very weird bug.
> > >>>>>>Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > >>>>>>of vector, and kernel reports successful read/write.
[...]
> The discussion is mainly around whether USER_DS for 32-bit compat apps
> should be the same as USER_DS for native 32-bit apps. Even for native
> 32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
> 0xffffffff would fail in both cases anyway. I think the LTP test doesn't
> even try to access such memory but only to probe the range validity (I
> haven't managed to build the latest LTP yet).

OK, so I managed to get the latest LTP (tag 20160510) built for AArch32
but the preadv02 does not fail:

-----------------------------
# uname -m
aarch64

# file ./testcases/bin/preadv02
./testcases/bin/preadv02: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 2.6.32, BuildID[sha1]=98066291426e1d3ee49d8504ce2a5bd721ab7fe8, not stripped

# ./testcases/bin/preadv02
preadv02.c:97: PASS: preadv() failed as expected: EINVAL
preadv02.c:97: PASS: preadv() failed as expected: EINVAL
preadv02.c:97: PASS: preadv() failed as expected: EINVAL
preadv02.c:97: PASS: preadv() failed as expected: EFAULT
preadv02.c:97: PASS: preadv() failed as expected: EBADF
preadv02.c:97: PASS: preadv() failed as expected: EBADF
preadv02.c:97: PASS: preadv() failed as expected: EISDIR
preadv02.c:97: PASS: preadv() failed as expected: ESPIPE

Summary:
passed   8
failed   0
skipped  0
warnings 0
-----------------------------

It's the 4th test above which passes iovec_base as -1 and expects
EFAULT. However, it seems to get the expected error. I guess that even
if access_ok() passes, the access to that location by the kernel would
fail since there isn't anything mapped.

With ILP32, however, STACK_TOP is TASK_SIZE and you may have the address
mapped already. I still think the USER_DS fix is useful, though I need
another way to check that it's actually a fix.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 01:51:15PM +0300, Yury Norov wrote:
> On Fri, May 13, 2016 at 09:28:03AM +0000, Catalin Marinas wrote:
> > The discussion is mainly around whether USER_DS for 32-bit compat apps
> > should be the same as USER_DS for native 32-bit apps. Even for native
> > 32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
> > 0xffffffff would fail in both cases anyway. I think the LTP test doesn't
> > even try to access such memory but only to probe the range validity (I
> > haven't managed to build the latest LTP yet).
> 
> This fix lets me build it (on top of 7b3ef3b0b)
> Of course, it's not 'official'. :)
[...]
> --- a/testcases/kernel/syscalls/fstatat/fstatat01.c
> +++ b/testcases/kernel/syscalls/fstatat/fstatat01.c
> @@ -59,6 +59,7 @@ static const char *filenames[TEST_CASES];
>  static const int expected_errno[] = { 0, 0, ENOTDIR, EBADF, EINVAL, 0 };
>  static const int flags[] = { 0, 0, 0, 0, 9999, 0 };
>  
> +#define HAVE_FSTATAT
>  #if !defined(HAVE_FSTATAT)
>  #if (__NR_fstatat64 > 0)
>  int fstatat(int dirfd, const char *filename, struct stat64 *statbuf, int flags)
> diff --git a/testcases/kernel/syscalls/preadv/preadv.h b/testcases/kernel/syscalls/preadv/preadv.h
> index f3ac30d..b001389 100644
> --- a/testcases/kernel/syscalls/preadv/preadv.h
> +++ b/testcases/kernel/syscalls/preadv/preadv.h
> @@ -21,6 +21,7 @@
>  #include "config.h"
>  #include "linux_syscall_numbers.h"
>  
> +#define HAVE_PREADV
>  #if !defined(HAVE_PREADV)
>  int preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
>  {
> diff --git a/testcases/kernel/syscalls/pwritev/pwritev.h b/testcases/kernel/syscalls/pwritev/pwritev.h
> index ae9d999..2a4d188 100644
> --- a/testcases/kernel/syscalls/pwritev/pwritev.h
> +++ b/testcases/kernel/syscalls/pwritev/pwritev.h
> @@ -21,6 +21,7 @@
>  #include "config.h"
>  #include "linux_syscall_numbers.h"
>  
> +#define HAVE_PWRITEV
>  #if !defined(HAVE_PWRITEV)
>  int pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
>  {

I didn't need any of these defines as they are automatically generated
in include/config.h for AArch32. However, I need to run autoreconf to
generate the configure script prior to building (archlinuxarm
filesystem)

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Fri, May 13, 2016 at 09:28:03AM +0000, Catalin Marinas wrote:
> 
> The discussion is mainly around whether USER_DS for 32-bit compat apps
> should be the same as USER_DS for native 32-bit apps. Even for native
> 32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
> 0xffffffff would fail in both cases anyway. I think the LTP test doesn't
> even try to access such memory but only to probe the range validity (I
> haven't managed to build the latest LTP yet).

This fix lets me build it (on top of 7b3ef3b0b)
Of course, it's not 'official'. :)

---
 testcases/kernel/syscalls/fstatat/fstatat01.c  | 1 +
 testcases/kernel/syscalls/preadv/preadv.h      | 1 +
 testcases/kernel/syscalls/pwritev/pwritev.h    | 1 +
 testcases/kernel/syscalls/request_key/Makefile | 2 +-
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fstatat/fstatat01.c b/testcases/kernel/syscalls/fstatat/fstatat01.c
index 128f6dd..6e23c9e 100644
--- a/testcases/kernel/syscalls/fstatat/fstatat01.c
+++ b/testcases/kernel/syscalls/fstatat/fstatat01.c
@@ -59,6 +59,7 @@ static const char *filenames[TEST_CASES];
 static const int expected_errno[] = { 0, 0, ENOTDIR, EBADF, EINVAL, 0 };
 static const int flags[] = { 0, 0, 0, 0, 9999, 0 };
 
+#define HAVE_FSTATAT
 #if !defined(HAVE_FSTATAT)
 #if (__NR_fstatat64 > 0)
 int fstatat(int dirfd, const char *filename, struct stat64 *statbuf, int flags)
diff --git a/testcases/kernel/syscalls/preadv/preadv.h b/testcases/kernel/syscalls/preadv/preadv.h
index f3ac30d..b001389 100644
--- a/testcases/kernel/syscalls/preadv/preadv.h
+++ b/testcases/kernel/syscalls/preadv/preadv.h
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "linux_syscall_numbers.h"
 
+#define HAVE_PREADV
 #if !defined(HAVE_PREADV)
 int preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 {
diff --git a/testcases/kernel/syscalls/pwritev/pwritev.h b/testcases/kernel/syscalls/pwritev/pwritev.h
index ae9d999..2a4d188 100644
--- a/testcases/kernel/syscalls/pwritev/pwritev.h
+++ b/testcases/kernel/syscalls/pwritev/pwritev.h
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "linux_syscall_numbers.h"
 
+#define HAVE_PWRITEV
 #if !defined(HAVE_PWRITEV)
 int pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 {
diff --git a/testcases/kernel/syscalls/request_key/Makefile b/testcases/kernel/syscalls/request_key/Makefile
index 9add429..2e8a37c 100644
--- a/testcases/kernel/syscalls/request_key/Makefile
+++ b/testcases/kernel/syscalls/request_key/Makefile
@@ -19,6 +19,6 @@ top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-LDLIBS		+= $(KEYUTILS_LIBS)
+LDLIBS		+= $(lkeyutils)
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
-- 
2.5.0


> 
> -- 
> Catalin
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 04:11:23PM +0800, Zhangjian (Bamvor) wrote:
> On 2016/5/12 23:28, Catalin Marinas wrote:
> >On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
> >>On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> >>>On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> >>>>On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> >>>>>On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> >>>>>>I debugged preadv02 and pwritev02 failures and found very weird bug.
> >>>>>>Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> >>>>>>of vector, and kernel reports successful read/write.
> >>>>>>
> >>>>>>There are 2 problems:
> >>>>>>1. How kernel allows such address to be passed to fs subsystem;
> >>>>>>2. How fs successes to read/write at non-mapped, and in fact non-user
> >>>>>>address.
> >>>>>>
> >>>>>>I don't know the answer on 2'nd question, and it might be something
> >>>>>>generic. But I investigated first problem.
> >>>>>>
> >>>>>>The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> >>>>>>validate user address, and on arm64 it ends up with checking buffer
> >>>>>>end against current_thread_info()->addr_limit.
> >>>>>>
> >>>>>>current_thread_info()->addr_limit for ilp32, and most probably for
> >>>>>>aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> >>>>>>It happens because on thread creation we call flush_old_exec() to set
> >>>>>>addr_limit, and completely ignore compat mode there.
[...]
> >>>>That's true, but USER_DS depends on personality which is not set yet
> >>>>for new thread, as I wrote above. In fact, I tried correct USER_DS
> >>>>only, and it doesn't work
> >>>
> >>>Ah, it looks like load_elf_binary() sets the personality after
> >>>flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> >>>maximum 64-bit task value, so they should have a similar issue with
> >>>native 32-bit vs compat behaviour.
[...]
> >>>So what exactly is LTP complaining about? Is different error (like
> >>>EFAULT vs EINVAL) or not getting an error at all.
> >>
> >>It should be EINVAL, but it succeed. The other problem is that
> >>following fs routines does not complain on wrong address.
> >
> >I see. The test asks the kernel to write a single byte (out of maximum
> >64) to the user address 0xffffffff.
> 
> What address We should set for this limitation, TASK_SIZE or STACK_TOP?
> It is same for 64bit application. But STACK_TOP(0xffff0000) is below
> TASK_SIZE in 32bit application. The address above STACK_TOP is preserved
> for 32bit application.

The discussion is mainly around whether USER_DS for 32-bit compat apps
should be the same as USER_DS for native 32-bit apps. Even for native
32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
0xffffffff would fail in both cases anyway. I think the LTP test doesn't
even try to access such memory but only to probe the range validity (I
haven't managed to build the latest LTP yet).

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 04:11:23PM +0800, Zhangjian (Bamvor) wrote:
> On 2016/5/12 23:28, Catalin Marinas wrote:
> >On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
> >>On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> >>>On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> >>>>On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> >>>>>On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> >>>>>>I debugged preadv02 and pwritev02 failures and found very weird bug.
> >>>>>>Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> >>>>>>of vector, and kernel reports successful read/write.
> >>>>>>
> >>>>>>There are 2 problems:
> >>>>>>1. How kernel allows such address to be passed to fs subsystem;
> >>>>>>2. How fs successes to read/write at non-mapped, and in fact non-user
> >>>>>>address.
> >>>>>>
> >>>>>>I don't know the answer on 2'nd question, and it might be something
> >>>>>>generic. But I investigated first problem.
> >>>>>>
> >>>>>>The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> >>>>>>validate user address, and on arm64 it ends up with checking buffer
> >>>>>>end against current_thread_info()->addr_limit.
> >>>>>>
> >>>>>>current_thread_info()->addr_limit for ilp32, and most probably for
> >>>>>>aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> >>>>>>It happens because on thread creation we call flush_old_exec() to set
> >>>>>>addr_limit, and completely ignore compat mode there.
[...]
> >>>>That's true, but USER_DS depends on personality which is not set yet
> >>>>for new thread, as I wrote above. In fact, I tried correct USER_DS
> >>>>only, and it doesn't work
> >>>
> >>>Ah, it looks like load_elf_binary() sets the personality after
> >>>flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> >>>maximum 64-bit task value, so they should have a similar issue with
> >>>native 32-bit vs compat behaviour.
[...]
> >>>So what exactly is LTP complaining about? Is different error (like
> >>>EFAULT vs EINVAL) or not getting an error at all.
> >>
> >>It should be EINVAL, but it succeed. The other problem is that
> >>following fs routines does not complain on wrong address.
> >
> >I see. The test asks the kernel to write a single byte (out of maximum
> >64) to the user address 0xffffffff.
> 
> What address We should set for this limitation, TASK_SIZE or STACK_TOP?
> It is same for 64bit application. But STACK_TOP(0xffff0000) is below
> TASK_SIZE in 32bit application. The address above STACK_TOP is preserved
> for 32bit application.

The discussion is mainly around whether USER_DS for 32-bit compat apps
should be the same as USER_DS for native 32-bit apps. Even for native
32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
0xffffffff would fail in both cases anyway. I think the LTP test doesn't
even try to access such memory but only to probe the range validity (I
haven't managed to build the latest LTP yet).

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Zhangjian (Bamvor)]

Hi,

On 2016/5/12 23:28, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
>> On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
>>> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
>>>> On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
>>>>> On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
>>>>>> I debugged preadv02 and pwritev02 failures and found very weird bug.
>>>>>> Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
>>>>>> of vector, and kernel reports successful read/write.
>>>>>>
>>>>>> There are 2 problems:
>>>>>> 1. How kernel allows such address to be passed to fs subsystem;
>>>>>> 2. How fs successes to read/write at non-mapped, and in fact non-user
>>>>>> address.
>>>>>>
>>>>>> I don't know the answer on 2'nd question, and it might be something
>>>>>> generic. But I investigated first problem.
>>>>>>
>>>>>> The problem is that compat_rw_copy_check_uvector() uses access_ok() to
>>>>>> validate user address, and on arm64 it ends up with checking buffer
>>>>>> end against current_thread_info()->addr_limit.
>>>>>>
>>>>>> current_thread_info()->addr_limit for ilp32, and most probably for
>>>>>> aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
>>>>>> It happens because on thread creation we call flush_old_exec() to set
>>>>>> addr_limit, and completely ignore compat mode there.
>>>
>>> [...]
>>>
>>>>>> --- a/arch/arm64/kernel/binfmt_elf32.c
>>>>>> +++ b/arch/arm64/kernel/binfmt_elf32.c
>>>>>> @@ -12,6 +12,7 @@
>>>>>>   do {						\
>>>>>>   	clear_thread_flag(TIF_32BIT_AARCH64);	\
>>>>>>   	set_thread_flag(TIF_32BIT);		\
>>>>>> +	set_fs(TASK_SIZE_32);			\
>>>>>>   } while (0)
>>>>>>
>>>>>>   #define COMPAT_ARCH_DLINFO
>>>>>> diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
>>>>>> index a934fd4..a8599c6 100644
>>>>>> --- a/arch/arm64/kernel/binfmt_ilp32.c
>>>>>> +++ b/arch/arm64/kernel/binfmt_ilp32.c
>>>>>> @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
>>>>>>   do {									\
>>>>>>   	set_thread_flag(TIF_32BIT_AARCH64);				\
>>>>>>   	clear_thread_flag(TIF_32BIT);					\
>>>>>> +	set_fs(TASK_SIZE_32);						\
>>>>>>   } while (0)
>>>>>
>>>>> I don't think we need these two. AFAICT, flush_old_exec() takes care of
>>>>> setting the USER_DS for the new thread.
>>>>
>>>> That's true, but USER_DS depends on personality which is not set yet
>>>> for new thread, as I wrote above. In fact, I tried correct USER_DS
>>>> only, and it doesn't work
>>>
>>> Ah, it looks like load_elf_binary() sets the personality after
>>> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
>>> maximum 64-bit task value, so they should have a similar issue with
>>> native 32-bit vs compat behaviour.
>>
>> Hmmm. If so, it means we'd introduce generic fix. It would be removing
>> set_fs() from flush_old_exec() and appending it to load_elf_binary()
>> after SET_PERSONALITY(). But I think it should be agreed with other
>> arches developers.
>
> The set_fs() in flush_old_exec() is probably fine, it may be meant to
> re-set the USER_DS for the old thread.
>
> It appears that at least powerpc and x86 don't have different USER_DS
> setting for native and compat, so moving the set_fs() call further down
> would not make any difference for them, nor will it fix the preadv02 LTP
> test (if it fails for them, I haven't checked).
>
>> I've sent standalone patch for aarch64 (you in CC) so let's move
>> discussion there.
>
> I've seen the patch but we would lose some discussion history here. I
> think we should continue this thread and just summarise the conclusion
> in reply to the other patch. This thread is also available on
> linux-arch, in case other architecture maintainers follow it.
>
>>> So what exactly is LTP complaining about? Is different error (like
>>> EFAULT vs EINVAL) or not getting an error at all.
>>
>> It should be EINVAL, but it succeed. The other problem is that
>> following fs routines does not complain on wrong address.
>
> I see. The test asks the kernel to write a single byte (out of maximum
> 64) to the user address 0xffffffff.
What address We should set for this limitation, TASK_SIZE or STACK_TOP?
It is same for 64bit application. But STACK_TOP(0xffff0000) is below
TASK_SIZE in 32bit application. The address above STACK_TOP is preserved
for 32bit application.

Regards

Bamvor

 > In the absence of the access_ok()
> check, this operation succeeds. If the preadv syscall gets 2 bytes as
> the count, then it would fail with EFAULT.
>
> While it's not really a bug, I agree that for matching the native 32-bit
> behavior (basically for other syscalls like those involving vfs_read()),
> the simplest fix would be to have a dynamic USER_DS.



>

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Zhangjian (Bamvor)]

Hi,

On 2016/5/12 23:28, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
>> On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
>>> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
>>>> On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
>>>>> On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
>>>>>> I debugged preadv02 and pwritev02 failures and found very weird bug.
>>>>>> Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
>>>>>> of vector, and kernel reports successful read/write.
>>>>>>
>>>>>> There are 2 problems:
>>>>>> 1. How kernel allows such address to be passed to fs subsystem;
>>>>>> 2. How fs successes to read/write at non-mapped, and in fact non-user
>>>>>> address.
>>>>>>
>>>>>> I don't know the answer on 2'nd question, and it might be something
>>>>>> generic. But I investigated first problem.
>>>>>>
>>>>>> The problem is that compat_rw_copy_check_uvector() uses access_ok() to
>>>>>> validate user address, and on arm64 it ends up with checking buffer
>>>>>> end against current_thread_info()->addr_limit.
>>>>>>
>>>>>> current_thread_info()->addr_limit for ilp32, and most probably for
>>>>>> aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
>>>>>> It happens because on thread creation we call flush_old_exec() to set
>>>>>> addr_limit, and completely ignore compat mode there.
>>>
>>> [...]
>>>
>>>>>> --- a/arch/arm64/kernel/binfmt_elf32.c
>>>>>> +++ b/arch/arm64/kernel/binfmt_elf32.c
>>>>>> @@ -12,6 +12,7 @@
>>>>>>   do {						\
>>>>>>   	clear_thread_flag(TIF_32BIT_AARCH64);	\
>>>>>>   	set_thread_flag(TIF_32BIT);		\
>>>>>> +	set_fs(TASK_SIZE_32);			\
>>>>>>   } while (0)
>>>>>>
>>>>>>   #define COMPAT_ARCH_DLINFO
>>>>>> diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
>>>>>> index a934fd4..a8599c6 100644
>>>>>> --- a/arch/arm64/kernel/binfmt_ilp32.c
>>>>>> +++ b/arch/arm64/kernel/binfmt_ilp32.c
>>>>>> @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
>>>>>>   do {									\
>>>>>>   	set_thread_flag(TIF_32BIT_AARCH64);				\
>>>>>>   	clear_thread_flag(TIF_32BIT);					\
>>>>>> +	set_fs(TASK_SIZE_32);						\
>>>>>>   } while (0)
>>>>>
>>>>> I don't think we need these two. AFAICT, flush_old_exec() takes care of
>>>>> setting the USER_DS for the new thread.
>>>>
>>>> That's true, but USER_DS depends on personality which is not set yet
>>>> for new thread, as I wrote above. In fact, I tried correct USER_DS
>>>> only, and it doesn't work
>>>
>>> Ah, it looks like load_elf_binary() sets the personality after
>>> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
>>> maximum 64-bit task value, so they should have a similar issue with
>>> native 32-bit vs compat behaviour.
>>
>> Hmmm. If so, it means we'd introduce generic fix. It would be removing
>> set_fs() from flush_old_exec() and appending it to load_elf_binary()
>> after SET_PERSONALITY(). But I think it should be agreed with other
>> arches developers.
>
> The set_fs() in flush_old_exec() is probably fine, it may be meant to
> re-set the USER_DS for the old thread.
>
> It appears that at least powerpc and x86 don't have different USER_DS
> setting for native and compat, so moving the set_fs() call further down
> would not make any difference for them, nor will it fix the preadv02 LTP
> test (if it fails for them, I haven't checked).
>
>> I've sent standalone patch for aarch64 (you in CC) so let's move
>> discussion there.
>
> I've seen the patch but we would lose some discussion history here. I
> think we should continue this thread and just summarise the conclusion
> in reply to the other patch. This thread is also available on
> linux-arch, in case other architecture maintainers follow it.
>
>>> So what exactly is LTP complaining about? Is different error (like
>>> EFAULT vs EINVAL) or not getting an error at all.
>>
>> It should be EINVAL, but it succeed. The other problem is that
>> following fs routines does not complain on wrong address.
>
> I see. The test asks the kernel to write a single byte (out of maximum
> 64) to the user address 0xffffffff.
What address We should set for this limitation, TASK_SIZE or STACK_TOP?
It is same for 64bit application. But STACK_TOP(0xffff0000) is below
TASK_SIZE in 32bit application. The address above STACK_TOP is preserved
for 32bit application.

Regards

Bamvor

 > In the absence of the access_ok()
> check, this operation succeeds. If the preadv syscall gets 2 bytes as
> the count, then it would fail with EFAULT.
>
> While it's not really a bug, I agree that for matching the native 32-bit
> behavior (basically for other syscalls like those involving vfs_read()),
> the simplest fix would be to have a dynamic USER_DS.



>

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
> On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > > of vector, and kernel reports successful read/write.
> > > > > 
> > > > > There are 2 problems:
> > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > address.
> > > > > 
> > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > generic. But I investigated first problem.
> > > > > 
> > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > end against current_thread_info()->addr_limit.
> > > > > 
> > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > addr_limit, and completely ignore compat mode there.
> > 
> > [...]
> > 
> > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > @@ -12,6 +12,7 @@
> > > > >  do {						\
> > > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > > >  	set_thread_flag(TIF_32BIT);		\
> > > > > +	set_fs(TASK_SIZE_32);			\
> > > > >  } while (0)
> > > > >  
> > > > >  #define COMPAT_ARCH_DLINFO
> > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > index a934fd4..a8599c6 100644
> > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > > >  do {									\
> > > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > > +	set_fs(TASK_SIZE_32);						\
> > > > >  } while (0)
> > > > 
> > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > setting the USER_DS for the new thread.
> > > 
> > > That's true, but USER_DS depends on personality which is not set yet
> > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > only, and it doesn't work
> > 
> > Ah, it looks like load_elf_binary() sets the personality after
> > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > maximum 64-bit task value, so they should have a similar issue with
> > native 32-bit vs compat behaviour.
> 
> Hmmm. If so, it means we'd introduce generic fix. It would be removing 
> set_fs() from flush_old_exec() and appending it to load_elf_binary()
> after SET_PERSONALITY(). But I think it should be agreed with other
> arches developers.

The set_fs() in flush_old_exec() is probably fine, it may be meant to
re-set the USER_DS for the old thread.

It appears that at least powerpc and x86 don't have different USER_DS
setting for native and compat, so moving the set_fs() call further down
would not make any difference for them, nor will it fix the preadv02 LTP
test (if it fails for them, I haven't checked).

> I've sent standalone patch for aarch64 (you in CC) so let's move
> discussion there.

I've seen the patch but we would lose some discussion history here. I
think we should continue this thread and just summarise the conclusion
in reply to the other patch. This thread is also available on
linux-arch, in case other architecture maintainers follow it.

> > So what exactly is LTP complaining about? Is different error (like
> > EFAULT vs EINVAL) or not getting an error at all.
>  
> It should be EINVAL, but it succeed. The other problem is that
> following fs routines does not complain on wrong address.

I see. The test asks the kernel to write a single byte (out of maximum
64) to the user address 0xffffffff. In the absence of the access_ok()
check, this operation succeeds. If the preadv syscall gets 2 bytes as
the count, then it would fail with EFAULT.

While it's not really a bug, I agree that for matching the native 32-bit
behavior (basically for other syscalls like those involving vfs_read()),
the simplest fix would be to have a dynamic USER_DS.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 03:54:03PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 05:34:15PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 03:20:16PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > > > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > > > > of vector, and kernel reports successful read/write.
> > > > > > > 
> > > > > > > There are 2 problems:
> > > > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > > > address.
> > > > > > > 
> > > > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > > > generic. But I investigated first problem.
> > > > > > > 
> > > > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > > > end against current_thread_info()->addr_limit.
> > > > > > > 
> > > > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > > > addr_limit, and completely ignore compat mode there.
> > > > 
> > > > [...]
> > > > 
> > > > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > > > @@ -12,6 +12,7 @@
> > > > > > >  do {						\
> > > > > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > > > > >  	set_thread_flag(TIF_32BIT);		\
> > > > > > > +	set_fs(TASK_SIZE_32);			\
> > > > > > >  } while (0)
> > > > > > >  
> > > > > > >  #define COMPAT_ARCH_DLINFO
> > > > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > > index a934fd4..a8599c6 100644
> > > > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > > > > >  do {									\
> > > > > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > > > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > > > > +	set_fs(TASK_SIZE_32);						\
> > > > > > >  } while (0)
> > > > > > 
> > > > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > > > setting the USER_DS for the new thread.
> > > > > 
> > > > > That's true, but USER_DS depends on personality which is not set yet
> > > > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > > > only, and it doesn't work
> > > > 
> > > > Ah, it looks like load_elf_binary() sets the personality after
> > > > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > > > maximum 64-bit task value, so they should have a similar issue with
> > > > native 32-bit vs compat behaviour.
> > > 
> > > I think we have another problem. flush_old_exec() calls the arm64
> > > flush_thread() where tls_thread_flush() checks for is_compat_task(). So
> > > starting a 32-bit application from a 64-bit one not go on the correct
> > > path.
> > 
> > As per now, all native, aarch32 and ilp32 tasks can exec() any
> > binaries they need.
> 
> And that's correct.
> 
> > Are you think it's wrong? If so, how we coild run
> > first compat application (maybe shell), it there are only lp64 tasks
> > on the system?
> 
> What I meant is that we rely on flush_old_exec() to initialise the TLS
> register for the compat task but it currently depends on what the parent
> is. I think tls_thread_flush() should actually drop the is_compat_task()
> thread and always initialise all the TLS registers.
> 

OK. I'll do it in v2.

> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 03:54:03PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 05:34:15PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 03:20:16PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > > > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > > > > of vector, and kernel reports successful read/write.
> > > > > > > 
> > > > > > > There are 2 problems:
> > > > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > > > address.
> > > > > > > 
> > > > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > > > generic. But I investigated first problem.
> > > > > > > 
> > > > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > > > end against current_thread_info()->addr_limit.
> > > > > > > 
> > > > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > > > addr_limit, and completely ignore compat mode there.
> > > > 
> > > > [...]
> > > > 
> > > > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > > > @@ -12,6 +12,7 @@
> > > > > > >  do {						\
> > > > > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > > > > >  	set_thread_flag(TIF_32BIT);		\
> > > > > > > +	set_fs(TASK_SIZE_32);			\
> > > > > > >  } while (0)
> > > > > > >  
> > > > > > >  #define COMPAT_ARCH_DLINFO
> > > > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > > index a934fd4..a8599c6 100644
> > > > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > > > > >  do {									\
> > > > > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > > > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > > > > +	set_fs(TASK_SIZE_32);						\
> > > > > > >  } while (0)
> > > > > > 
> > > > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > > > setting the USER_DS for the new thread.
> > > > > 
> > > > > That's true, but USER_DS depends on personality which is not set yet
> > > > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > > > only, and it doesn't work
> > > > 
> > > > Ah, it looks like load_elf_binary() sets the personality after
> > > > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > > > maximum 64-bit task value, so they should have a similar issue with
> > > > native 32-bit vs compat behaviour.
> > > 
> > > I think we have another problem. flush_old_exec() calls the arm64
> > > flush_thread() where tls_thread_flush() checks for is_compat_task(). So
> > > starting a 32-bit application from a 64-bit one not go on the correct
> > > path.
> > 
> > As per now, all native, aarch32 and ilp32 tasks can exec() any
> > binaries they need.
> 
> And that's correct.
> 
> > Are you think it's wrong? If so, how we coild run
> > first compat application (maybe shell), it there are only lp64 tasks
> > on the system?
> 
> What I meant is that we rely on flush_old_exec() to initialise the TLS
> register for the compat task but it currently depends on what the parent
> is. I think tls_thread_flush() should actually drop the is_compat_task()
> thread and always initialise all the TLS registers.
> 

OK. I'll do it in v2.

> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 05:34:15PM +0300, Yury Norov wrote:
> On Thu, May 12, 2016 at 03:20:16PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > > > of vector, and kernel reports successful read/write.
> > > > > > 
> > > > > > There are 2 problems:
> > > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > > address.
> > > > > > 
> > > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > > generic. But I investigated first problem.
> > > > > > 
> > > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > > end against current_thread_info()->addr_limit.
> > > > > > 
> > > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > > addr_limit, and completely ignore compat mode there.
> > > 
> > > [...]
> > > 
> > > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > > @@ -12,6 +12,7 @@
> > > > > >  do {						\
> > > > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > > > >  	set_thread_flag(TIF_32BIT);		\
> > > > > > +	set_fs(TASK_SIZE_32);			\
> > > > > >  } while (0)
> > > > > >  
> > > > > >  #define COMPAT_ARCH_DLINFO
> > > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > index a934fd4..a8599c6 100644
> > > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > > > >  do {									\
> > > > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > > > +	set_fs(TASK_SIZE_32);						\
> > > > > >  } while (0)
> > > > > 
> > > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > > setting the USER_DS for the new thread.
> > > > 
> > > > That's true, but USER_DS depends on personality which is not set yet
> > > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > > only, and it doesn't work
> > > 
> > > Ah, it looks like load_elf_binary() sets the personality after
> > > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > > maximum 64-bit task value, so they should have a similar issue with
> > > native 32-bit vs compat behaviour.
> > 
> > I think we have another problem. flush_old_exec() calls the arm64
> > flush_thread() where tls_thread_flush() checks for is_compat_task(). So
> > starting a 32-bit application from a 64-bit one not go on the correct
> > path.
> 
> As per now, all native, aarch32 and ilp32 tasks can exec() any
> binaries they need.

And that's correct.

> Are you think it's wrong? If so, how we coild run
> first compat application (maybe shell), it there are only lp64 tasks
> on the system?

What I meant is that we rely on flush_old_exec() to initialise the TLS
register for the compat task but it currently depends on what the parent
is. I think tls_thread_flush() should actually drop the is_compat_task()
thread and always initialise all the TLS registers.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 03:20:16PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > > of vector, and kernel reports successful read/write.
> > > > > 
> > > > > There are 2 problems:
> > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > address.
> > > > > 
> > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > generic. But I investigated first problem.
> > > > > 
> > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > end against current_thread_info()->addr_limit.
> > > > > 
> > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > addr_limit, and completely ignore compat mode there.
> > 
> > [...]
> > 
> > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > @@ -12,6 +12,7 @@
> > > > >  do {						\
> > > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > > >  	set_thread_flag(TIF_32BIT);		\
> > > > > +	set_fs(TASK_SIZE_32);			\
> > > > >  } while (0)
> > > > >  
> > > > >  #define COMPAT_ARCH_DLINFO
> > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > index a934fd4..a8599c6 100644
> > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > > >  do {									\
> > > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > > +	set_fs(TASK_SIZE_32);						\
> > > > >  } while (0)
> > > > 
> > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > setting the USER_DS for the new thread.
> > > 
> > > That's true, but USER_DS depends on personality which is not set yet
> > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > only, and it doesn't work
> > 
> > Ah, it looks like load_elf_binary() sets the personality after
> > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > maximum 64-bit task value, so they should have a similar issue with
> > native 32-bit vs compat behaviour.
> 
> I think we have another problem. flush_old_exec() calls the arm64
> flush_thread() where tls_thread_flush() checks for is_compat_task(). So
> starting a 32-bit application from a 64-bit one not go on the correct
> path.

As per now, all native, aarch32 and ilp32 tasks can exec() any
binaries they need. Are you think it's wrong? If so, how we coild run
first compat application (maybe shell), it there are only lp64 tasks
on the system?

> 
> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 03:20:16PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > > of vector, and kernel reports successful read/write.
> > > > > 
> > > > > There are 2 problems:
> > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > address.
> > > > > 
> > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > generic. But I investigated first problem.
> > > > > 
> > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > end against current_thread_info()->addr_limit.
> > > > > 
> > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > addr_limit, and completely ignore compat mode there.
> > 
> > [...]
> > 
> > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > @@ -12,6 +12,7 @@
> > > > >  do {						\
> > > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > > >  	set_thread_flag(TIF_32BIT);		\
> > > > > +	set_fs(TASK_SIZE_32);			\
> > > > >  } while (0)
> > > > >  
> > > > >  #define COMPAT_ARCH_DLINFO
> > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > index a934fd4..a8599c6 100644
> > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > > >  do {									\
> > > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > > +	set_fs(TASK_SIZE_32);						\
> > > > >  } while (0)
> > > > 
> > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > setting the USER_DS for the new thread.
> > > 
> > > That's true, but USER_DS depends on personality which is not set yet
> > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > only, and it doesn't work
> > 
> > Ah, it looks like load_elf_binary() sets the personality after
> > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > maximum 64-bit task value, so they should have a similar issue with
> > native 32-bit vs compat behaviour.
> 
> I think we have another problem. flush_old_exec() calls the arm64
> flush_thread() where tls_thread_flush() checks for is_compat_task(). So
> starting a 32-bit application from a 64-bit one not go on the correct
> path.

As per now, all native, aarch32 and ilp32 tasks can exec() any
binaries they need. Are you think it's wrong? If so, how we coild run
first compat application (maybe shell), it there are only lp64 tasks
on the system?

> 
> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > of vector, and kernel reports successful read/write.
> > > > 
> > > > There are 2 problems:
> > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > address.
> > > > 
> > > > I don't know the answer on 2'nd question, and it might be something
> > > > generic. But I investigated first problem.
> > > > 
> > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > validate user address, and on arm64 it ends up with checking buffer
> > > > end against current_thread_info()->addr_limit.
> > > > 
> > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > addr_limit, and completely ignore compat mode there.
> 
> [...]
> 
> > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > @@ -12,6 +12,7 @@
> > > >  do {						\
> > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > >  	set_thread_flag(TIF_32BIT);		\
> > > > +	set_fs(TASK_SIZE_32);			\
> > > >  } while (0)
> > > >  
> > > >  #define COMPAT_ARCH_DLINFO
> > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > index a934fd4..a8599c6 100644
> > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > >  do {									\
> > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > +	set_fs(TASK_SIZE_32);						\
> > > >  } while (0)
> > > 
> > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > setting the USER_DS for the new thread.
> > 
> > That's true, but USER_DS depends on personality which is not set yet
> > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > only, and it doesn't work
> 
> Ah, it looks like load_elf_binary() sets the personality after
> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> maximum 64-bit task value, so they should have a similar issue with
> native 32-bit vs compat behaviour.

Hmmm. If so, it means we'd introduce generic fix. It would be removing 
set_fs() from flush_old_exec() and appending it to load_elf_binary()
after SET_PERSONALITY(). But I think it should be agreed with other
arches developers. I've sent standalone patch for aarch64 (you in CC)
so let's move discussion there.

> So what exactly is LTP complaining about? Is different error (like
> EFAULT vs EINVAL) or not getting an error at all.
 
It should be EINVAL, but it succeed. The other problem is that
following fs routines does not complain on wrong address.

> (I need to update my LTP, the preadv tests appeared in December last
> year)
> 

preadv02 was extended with this testcase in April.

> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > of vector, and kernel reports successful read/write.
> > > > 
> > > > There are 2 problems:
> > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > address.
> > > > 
> > > > I don't know the answer on 2'nd question, and it might be something
> > > > generic. But I investigated first problem.
> > > > 
> > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > validate user address, and on arm64 it ends up with checking buffer
> > > > end against current_thread_info()->addr_limit.
> > > > 
> > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > addr_limit, and completely ignore compat mode there.
> 
> [...]
> 
> > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > @@ -12,6 +12,7 @@
> > > >  do {						\
> > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > >  	set_thread_flag(TIF_32BIT);		\
> > > > +	set_fs(TASK_SIZE_32);			\
> > > >  } while (0)
> > > >  
> > > >  #define COMPAT_ARCH_DLINFO
> > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > index a934fd4..a8599c6 100644
> > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > >  do {									\
> > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > +	set_fs(TASK_SIZE_32);						\
> > > >  } while (0)
> > > 
> > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > setting the USER_DS for the new thread.
> > 
> > That's true, but USER_DS depends on personality which is not set yet
> > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > only, and it doesn't work
> 
> Ah, it looks like load_elf_binary() sets the personality after
> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> maximum 64-bit task value, so they should have a similar issue with
> native 32-bit vs compat behaviour.

Hmmm. If so, it means we'd introduce generic fix. It would be removing 
set_fs() from flush_old_exec() and appending it to load_elf_binary()
after SET_PERSONALITY(). But I think it should be agreed with other
arches developers. I've sent standalone patch for aarch64 (you in CC)
so let's move discussion there.

> So what exactly is LTP complaining about? Is different error (like
> EFAULT vs EINVAL) or not getting an error at all.
 
It should be EINVAL, but it succeed. The other problem is that
following fs routines does not complain on wrong address.

> (I need to update my LTP, the preadv tests appeared in December last
> year)
> 

preadv02 was extended with this testcase in April.

> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > of vector, and kernel reports successful read/write.
> > > > 
> > > > There are 2 problems:
> > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > address.
> > > > 
> > > > I don't know the answer on 2'nd question, and it might be something
> > > > generic. But I investigated first problem.
> > > > 
> > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > validate user address, and on arm64 it ends up with checking buffer
> > > > end against current_thread_info()->addr_limit.
> > > > 
> > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > addr_limit, and completely ignore compat mode there.
> 
> [...]
> 
> > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > @@ -12,6 +12,7 @@
> > > >  do {						\
> > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > >  	set_thread_flag(TIF_32BIT);		\
> > > > +	set_fs(TASK_SIZE_32);			\
> > > >  } while (0)
> > > >  
> > > >  #define COMPAT_ARCH_DLINFO
> > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > index a934fd4..a8599c6 100644
> > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > >  do {									\
> > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > +	set_fs(TASK_SIZE_32);						\
> > > >  } while (0)
> > > 
> > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > setting the USER_DS for the new thread.
> > 
> > That's true, but USER_DS depends on personality which is not set yet
> > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > only, and it doesn't work
> 
> Ah, it looks like load_elf_binary() sets the personality after
> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> maximum 64-bit task value, so they should have a similar issue with
> native 32-bit vs compat behaviour.

I think we have another problem. flush_old_exec() calls the arm64
flush_thread() where tls_thread_flush() checks for is_compat_task(). So
starting a 32-bit application from a 64-bit one not go on the correct
path.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > > of vector, and kernel reports successful read/write.
> > > > 
> > > > There are 2 problems:
> > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > address.
> > > > 
> > > > I don't know the answer on 2'nd question, and it might be something
> > > > generic. But I investigated first problem.
> > > > 
> > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > validate user address, and on arm64 it ends up with checking buffer
> > > > end against current_thread_info()->addr_limit.
> > > > 
> > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > addr_limit, and completely ignore compat mode there.
> 
> [...]
> 
> > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > @@ -12,6 +12,7 @@
> > > >  do {						\
> > > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > > >  	set_thread_flag(TIF_32BIT);		\
> > > > +	set_fs(TASK_SIZE_32);			\
> > > >  } while (0)
> > > >  
> > > >  #define COMPAT_ARCH_DLINFO
> > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > > index a934fd4..a8599c6 100644
> > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > > >  do {									\
> > > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > > >  	clear_thread_flag(TIF_32BIT);					\
> > > > +	set_fs(TASK_SIZE_32);						\
> > > >  } while (0)
> > > 
> > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > setting the USER_DS for the new thread.
> > 
> > That's true, but USER_DS depends on personality which is not set yet
> > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > only, and it doesn't work
> 
> Ah, it looks like load_elf_binary() sets the personality after
> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> maximum 64-bit task value, so they should have a similar issue with
> native 32-bit vs compat behaviour.

I think we have another problem. flush_old_exec() calls the arm64
flush_thread() where tls_thread_flush() checks for is_compat_task(). So
starting a 32-bit application from a 64-bit one not go on the correct
path.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > of vector, and kernel reports successful read/write.
> > > 
> > > There are 2 problems:
> > > 1. How kernel allows such address to be passed to fs subsystem;
> > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > address.
> > > 
> > > I don't know the answer on 2'nd question, and it might be something
> > > generic. But I investigated first problem.
> > > 
> > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > validate user address, and on arm64 it ends up with checking buffer
> > > end against current_thread_info()->addr_limit.
> > > 
> > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > It happens because on thread creation we call flush_old_exec() to set 
> > > addr_limit, and completely ignore compat mode there.

[...]

> > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > @@ -12,6 +12,7 @@
> > >  do {						\
> > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > >  	set_thread_flag(TIF_32BIT);		\
> > > +	set_fs(TASK_SIZE_32);			\
> > >  } while (0)
> > >  
> > >  #define COMPAT_ARCH_DLINFO
> > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > index a934fd4..a8599c6 100644
> > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > >  do {									\
> > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > >  	clear_thread_flag(TIF_32BIT);					\
> > > +	set_fs(TASK_SIZE_32);						\
> > >  } while (0)
> > 
> > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > setting the USER_DS for the new thread.
> 
> That's true, but USER_DS depends on personality which is not set yet
> for new thread, as I wrote above. In fact, I tried correct USER_DS
> only, and it doesn't work

Ah, it looks like load_elf_binary() sets the personality after
flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
maximum 64-bit task value, so they should have a similar issue with
native 32-bit vs compat behaviour.

So what exactly is LTP complaining about? Is different error (like
EFAULT vs EINVAL) or not getting an error at all.

(I need to update my LTP, the preadv tests appeared in December last
year)

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > > of vector, and kernel reports successful read/write.
> > > 
> > > There are 2 problems:
> > > 1. How kernel allows such address to be passed to fs subsystem;
> > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > address.
> > > 
> > > I don't know the answer on 2'nd question, and it might be something
> > > generic. But I investigated first problem.
> > > 
> > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > validate user address, and on arm64 it ends up with checking buffer
> > > end against current_thread_info()->addr_limit.
> > > 
> > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > It happens because on thread creation we call flush_old_exec() to set 
> > > addr_limit, and completely ignore compat mode there.

[...]

> > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > @@ -12,6 +12,7 @@
> > >  do {						\
> > >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> > >  	set_thread_flag(TIF_32BIT);		\
> > > +	set_fs(TASK_SIZE_32);			\
> > >  } while (0)
> > >  
> > >  #define COMPAT_ARCH_DLINFO
> > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > > index a934fd4..a8599c6 100644
> > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> > >  do {									\
> > >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> > >  	clear_thread_flag(TIF_32BIT);					\
> > > +	set_fs(TASK_SIZE_32);						\
> > >  } while (0)
> > 
> > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > setting the USER_DS for the new thread.
> 
> That's true, but USER_DS depends on personality which is not set yet
> for new thread, as I wrote above. In fact, I tried correct USER_DS
> only, and it doesn't work

Ah, it looks like load_elf_binary() sets the personality after
flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
maximum 64-bit task value, so they should have a similar issue with
native 32-bit vs compat behaviour.

So what exactly is LTP complaining about? Is different error (like
EFAULT vs EINVAL) or not getting an error at all.

(I need to update my LTP, the preadv tests appeared in December last
year)

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > of vector, and kernel reports successful read/write.
> > 
> > There are 2 problems:
> > 1. How kernel allows such address to be passed to fs subsystem;
> > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > address.
> > 
> > I don't know the answer on 2'nd question, and it might be something
> > generic. But I investigated first problem.
> > 
> > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > validate user address, and on arm64 it ends up with checking buffer
> > end against current_thread_info()->addr_limit.
> > 
> > current_thread_info()->addr_limit for ilp32, and most probably for
> > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > It happens because on thread creation we call flush_old_exec() to set 
> > addr_limit, and completely ignore compat mode there.
> 
> I assume accesses beyond this address would fault anyway but I haven't
> checked the code paths.
> 
> > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> > index 7a39683..6ba4952 100644
> > --- a/arch/arm64/include/asm/elf.h
> > +++ b/arch/arm64/include/asm/elf.h
> > @@ -146,6 +146,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
> >  do {						\
> >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> >  	clear_thread_flag(TIF_32BIT);		\
> > +	set_fs(TASK_SIZE_64);			\
> >  } while (0)
> 
> See below.
> 
> > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > index 19cfdc5..3b0dd8d 100644
> > --- a/arch/arm64/include/asm/uaccess.h
> > +++ b/arch/arm64/include/asm/uaccess.h
> > @@ -51,7 +51,7 @@
> >  #define KERNEL_DS	(-1UL)
> >  #define get_ds()	(KERNEL_DS)
> >  
> > -#define USER_DS		TASK_SIZE_64
> > +#define USER_DS		TASK_SIZE
> 
> I agree with this.
> 
> >  #define get_fs()	(current_thread_info()->addr_limit)
> >  
> >  static inline void set_fs(mm_segment_t fs)
> > diff --git a/arch/arm64/kernel/binfmt_elf32.c b/arch/arm64/kernel/binfmt_elf32.c
> > index 5487872..2e8d9f3 100644
> > --- a/arch/arm64/kernel/binfmt_elf32.c
> > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > @@ -12,6 +12,7 @@
> >  do {						\
> >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> >  	set_thread_flag(TIF_32BIT);		\
> > +	set_fs(TASK_SIZE_32);			\
> >  } while (0)
> >  
> >  #define COMPAT_ARCH_DLINFO
> > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > index a934fd4..a8599c6 100644
> > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> >  do {									\
> >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> >  	clear_thread_flag(TIF_32BIT);					\
> > +	set_fs(TASK_SIZE_32);						\
> >  } while (0)
> 
> I don't think we need these two. AFAICT, flush_old_exec() takes care of
> setting the USER_DS for the new thread.

That's true, but USER_DS depends on personality which is not set yet
for new thread, as I wrote above. In fact, I tried correct USER_DS
only, and it doesn't work

> 
> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > of vector, and kernel reports successful read/write.
> > 
> > There are 2 problems:
> > 1. How kernel allows such address to be passed to fs subsystem;
> > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > address.
> > 
> > I don't know the answer on 2'nd question, and it might be something
> > generic. But I investigated first problem.
> > 
> > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > validate user address, and on arm64 it ends up with checking buffer
> > end against current_thread_info()->addr_limit.
> > 
> > current_thread_info()->addr_limit for ilp32, and most probably for
> > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > It happens because on thread creation we call flush_old_exec() to set 
> > addr_limit, and completely ignore compat mode there.
> 
> I assume accesses beyond this address would fault anyway but I haven't
> checked the code paths.
> 
> > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> > index 7a39683..6ba4952 100644
> > --- a/arch/arm64/include/asm/elf.h
> > +++ b/arch/arm64/include/asm/elf.h
> > @@ -146,6 +146,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
> >  do {						\
> >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> >  	clear_thread_flag(TIF_32BIT);		\
> > +	set_fs(TASK_SIZE_64);			\
> >  } while (0)
> 
> See below.
> 
> > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > index 19cfdc5..3b0dd8d 100644
> > --- a/arch/arm64/include/asm/uaccess.h
> > +++ b/arch/arm64/include/asm/uaccess.h
> > @@ -51,7 +51,7 @@
> >  #define KERNEL_DS	(-1UL)
> >  #define get_ds()	(KERNEL_DS)
> >  
> > -#define USER_DS		TASK_SIZE_64
> > +#define USER_DS		TASK_SIZE
> 
> I agree with this.
> 
> >  #define get_fs()	(current_thread_info()->addr_limit)
> >  
> >  static inline void set_fs(mm_segment_t fs)
> > diff --git a/arch/arm64/kernel/binfmt_elf32.c b/arch/arm64/kernel/binfmt_elf32.c
> > index 5487872..2e8d9f3 100644
> > --- a/arch/arm64/kernel/binfmt_elf32.c
> > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > @@ -12,6 +12,7 @@
> >  do {						\
> >  	clear_thread_flag(TIF_32BIT_AARCH64);	\
> >  	set_thread_flag(TIF_32BIT);		\
> > +	set_fs(TASK_SIZE_32);			\
> >  } while (0)
> >  
> >  #define COMPAT_ARCH_DLINFO
> > diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> > index a934fd4..a8599c6 100644
> > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> >  do {									\
> >  	set_thread_flag(TIF_32BIT_AARCH64);				\
> >  	clear_thread_flag(TIF_32BIT);					\
> > +	set_fs(TASK_SIZE_32);						\
> >  } while (0)
> 
> I don't think we need these two. AFAICT, flush_old_exec() takes care of
> setting the USER_DS for the new thread.

That's true, but USER_DS depends on personality which is not set yet
for new thread, as I wrote above. In fact, I tried correct USER_DS
only, and it doesn't work

> 
> -- 
> Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> I debugged preadv02 and pwritev02 failures and found very weird bug.
> Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> of vector, and kernel reports successful read/write.
> 
> There are 2 problems:
> 1. How kernel allows such address to be passed to fs subsystem;
> 2. How fs successes to read/write at non-mapped, and in fact non-user
> address.
> 
> I don't know the answer on 2'nd question, and it might be something
> generic. But I investigated first problem.
> 
> The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> validate user address, and on arm64 it ends up with checking buffer
> end against current_thread_info()->addr_limit.
> 
> current_thread_info()->addr_limit for ilp32, and most probably for
> aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> It happens because on thread creation we call flush_old_exec() to set 
> addr_limit, and completely ignore compat mode there.

I assume accesses beyond this address would fault anyway but I haven't
checked the code paths.

> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index 7a39683..6ba4952 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -146,6 +146,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>  do {						\
>  	clear_thread_flag(TIF_32BIT_AARCH64);	\
>  	clear_thread_flag(TIF_32BIT);		\
> +	set_fs(TASK_SIZE_64);			\
>  } while (0)

See below.

> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 19cfdc5..3b0dd8d 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -51,7 +51,7 @@
>  #define KERNEL_DS	(-1UL)
>  #define get_ds()	(KERNEL_DS)
>  
> -#define USER_DS		TASK_SIZE_64
> +#define USER_DS		TASK_SIZE

I agree with this.

>  #define get_fs()	(current_thread_info()->addr_limit)
>  
>  static inline void set_fs(mm_segment_t fs)
> diff --git a/arch/arm64/kernel/binfmt_elf32.c b/arch/arm64/kernel/binfmt_elf32.c
> index 5487872..2e8d9f3 100644
> --- a/arch/arm64/kernel/binfmt_elf32.c
> +++ b/arch/arm64/kernel/binfmt_elf32.c
> @@ -12,6 +12,7 @@
>  do {						\
>  	clear_thread_flag(TIF_32BIT_AARCH64);	\
>  	set_thread_flag(TIF_32BIT);		\
> +	set_fs(TASK_SIZE_32);			\
>  } while (0)
>  
>  #define COMPAT_ARCH_DLINFO
> diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> index a934fd4..a8599c6 100644
> --- a/arch/arm64/kernel/binfmt_ilp32.c
> +++ b/arch/arm64/kernel/binfmt_ilp32.c
> @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
>  do {									\
>  	set_thread_flag(TIF_32BIT_AARCH64);				\
>  	clear_thread_flag(TIF_32BIT);					\
> +	set_fs(TASK_SIZE_32);						\
>  } while (0)

I don't think we need these two. AFAICT, flush_old_exec() takes care of
setting the USER_DS for the new thread.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> I debugged preadv02 and pwritev02 failures and found very weird bug.
> Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> of vector, and kernel reports successful read/write.
> 
> There are 2 problems:
> 1. How kernel allows such address to be passed to fs subsystem;
> 2. How fs successes to read/write at non-mapped, and in fact non-user
> address.
> 
> I don't know the answer on 2'nd question, and it might be something
> generic. But I investigated first problem.
> 
> The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> validate user address, and on arm64 it ends up with checking buffer
> end against current_thread_info()->addr_limit.
> 
> current_thread_info()->addr_limit for ilp32, and most probably for
> aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> It happens because on thread creation we call flush_old_exec() to set 
> addr_limit, and completely ignore compat mode there.

I assume accesses beyond this address would fault anyway but I haven't
checked the code paths.

> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index 7a39683..6ba4952 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -146,6 +146,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
>  do {						\
>  	clear_thread_flag(TIF_32BIT_AARCH64);	\
>  	clear_thread_flag(TIF_32BIT);		\
> +	set_fs(TASK_SIZE_64);			\
>  } while (0)

See below.

> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 19cfdc5..3b0dd8d 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -51,7 +51,7 @@
>  #define KERNEL_DS	(-1UL)
>  #define get_ds()	(KERNEL_DS)
>  
> -#define USER_DS		TASK_SIZE_64
> +#define USER_DS		TASK_SIZE

I agree with this.

>  #define get_fs()	(current_thread_info()->addr_limit)
>  
>  static inline void set_fs(mm_segment_t fs)
> diff --git a/arch/arm64/kernel/binfmt_elf32.c b/arch/arm64/kernel/binfmt_elf32.c
> index 5487872..2e8d9f3 100644
> --- a/arch/arm64/kernel/binfmt_elf32.c
> +++ b/arch/arm64/kernel/binfmt_elf32.c
> @@ -12,6 +12,7 @@
>  do {						\
>  	clear_thread_flag(TIF_32BIT_AARCH64);	\
>  	set_thread_flag(TIF_32BIT);		\
> +	set_fs(TASK_SIZE_32);			\
>  } while (0)
>  
>  #define COMPAT_ARCH_DLINFO
> diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
> index a934fd4..a8599c6 100644
> --- a/arch/arm64/kernel/binfmt_ilp32.c
> +++ b/arch/arm64/kernel/binfmt_ilp32.c
> @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
>  do {									\
>  	set_thread_flag(TIF_32BIT_AARCH64);				\
>  	clear_thread_flag(TIF_32BIT);					\
> +	set_fs(TASK_SIZE_32);						\
>  } while (0)

I don't think we need these two. AFAICT, flush_old_exec() takes care of
setting the USER_DS for the new thread.

-- 
Catalin

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 11:19:21AM +0200, Arnd Bergmann wrote:
> On Thursday 12 May 2016 03:20:00 Yury Norov wrote:
> > 
> > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > of vector, and kernel reports successful read/write.
> > 
> > There are 2 problems:
> > 1. How kernel allows such address to be passed to fs subsystem;
> > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > address.
> > 
> > I don't know the answer on 2'nd question, and it might be something
> > generic. But I investigated first problem.
> > 
> > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > validate user address, and on arm64 it ends up with checking buffer
> > end against current_thread_info()->addr_limit.
> > 
> > current_thread_info()->addr_limit for ilp32, and most probably for
> > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > It happens because on thread creation we call flush_old_exec() to set 
> > addr_limit, and completely ignore compat mode there.
> > 
> > This patch fixes it. It also fixes USER_DS macro to return different
> > values depending on compat.
> > 
> > This patch is enough to handle preadv02 and pwritev02, but problem #2
> > is still there.
> > 
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > 
> 
> Good catch!
> 
> Can you do a version of this patch that works on the current
> mainline kernel and can be backported to fix aarch32 emulation?
> 
> For ilp32 mode, I think we can better fix arch/arm64/kernel/binfmt_ilp32.c
> as it is introduced.
> 
> 	Arnd
> 

OK, will do

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

On Thu, May 12, 2016 at 11:19:21AM +0200, Arnd Bergmann wrote:
> On Thursday 12 May 2016 03:20:00 Yury Norov wrote:
> > 
> > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> > of vector, and kernel reports successful read/write.
> > 
> > There are 2 problems:
> > 1. How kernel allows such address to be passed to fs subsystem;
> > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > address.
> > 
> > I don't know the answer on 2'nd question, and it might be something
> > generic. But I investigated first problem.
> > 
> > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > validate user address, and on arm64 it ends up with checking buffer
> > end against current_thread_info()->addr_limit.
> > 
> > current_thread_info()->addr_limit for ilp32, and most probably for
> > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > It happens because on thread creation we call flush_old_exec() to set 
> > addr_limit, and completely ignore compat mode there.
> > 
> > This patch fixes it. It also fixes USER_DS macro to return different
> > values depending on compat.
> > 
> > This patch is enough to handle preadv02 and pwritev02, but problem #2
> > is still there.
> > 
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > 
> 
> Good catch!
> 
> Can you do a version of this patch that works on the current
> mainline kernel and can be backported to fix aarch32 emulation?
> 
> For ilp32 mode, I think we can better fix arch/arm64/kernel/binfmt_ilp32.c
> as it is introduced.
> 
> 	Arnd
> 

OK, will do

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Arnd Bergmann]

On Thursday 12 May 2016 03:20:00 Yury Norov wrote:
> 
> I debugged preadv02 and pwritev02 failures and found very weird bug.
> Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
> of vector, and kernel reports successful read/write.
> 
> There are 2 problems:
> 1. How kernel allows such address to be passed to fs subsystem;
> 2. How fs successes to read/write at non-mapped, and in fact non-user
> address.
> 
> I don't know the answer on 2'nd question, and it might be something
> generic. But I investigated first problem.
> 
> The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> validate user address, and on arm64 it ends up with checking buffer
> end against current_thread_info()->addr_limit.
> 
> current_thread_info()->addr_limit for ilp32, and most probably for
> aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> It happens because on thread creation we call flush_old_exec() to set 
> addr_limit, and completely ignore compat mode there.
> 
> This patch fixes it. It also fixes USER_DS macro to return different
> values depending on compat.
> 
> This patch is enough to handle preadv02 and pwritev02, but problem #2
> is still there.
> 
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> 

Good catch!

Can you do a version of this patch that works on the current
mainline kernel and can be backported to fix aarch32 emulation?

For ilp32 mode, I think we can better fix arch/arm64/kernel/binfmt_ilp32.c
as it is introduced.

	Arnd

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

Hi,

I debugged preadv02 and pwritev02 failures and found very weird bug.
Test passes {iovec_base = 0xffffffff, iovec_len = 64} as one element
of vector, and kernel reports successful read/write.

There are 2 problems:
1. How kernel allows such address to be passed to fs subsystem;
2. How fs successes to read/write at non-mapped, and in fact non-user
address.

I don't know the answer on 2'nd question, and it might be something
generic. But I investigated first problem.

The problem is that compat_rw_copy_check_uvector() uses access_ok() to
validate user address, and on arm64 it ends up with checking buffer
end against current_thread_info()->addr_limit.

current_thread_info()->addr_limit for ilp32, and most probably for
aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
It happens because on thread creation we call flush_old_exec() to set 
addr_limit, and completely ignore compat mode there.

This patch fixes it. It also fixes USER_DS macro to return different
values depending on compat.

This patch is enough to handle preadv02 and pwritev02, but problem #2
is still there.

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/include/asm/elf.h     | 1 +
 arch/arm64/include/asm/uaccess.h | 2 +-
 arch/arm64/kernel/binfmt_elf32.c | 1 +
 arch/arm64/kernel/binfmt_ilp32.c | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 7a39683..6ba4952 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -146,6 +146,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 do {						\
 	clear_thread_flag(TIF_32BIT_AARCH64);	\
 	clear_thread_flag(TIF_32BIT);		\
+	set_fs(TASK_SIZE_64);			\
 } while (0)
 
 #define ARCH_DLINFO							\
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 19cfdc5..3b0dd8d 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -51,7 +51,7 @@
 #define KERNEL_DS	(-1UL)
 #define get_ds()	(KERNEL_DS)
 
-#define USER_DS		TASK_SIZE_64
+#define USER_DS		TASK_SIZE
 #define get_fs()	(current_thread_info()->addr_limit)
 
 static inline void set_fs(mm_segment_t fs)
diff --git a/arch/arm64/kernel/binfmt_elf32.c b/arch/arm64/kernel/binfmt_elf32.c
index 5487872..2e8d9f3 100644
--- a/arch/arm64/kernel/binfmt_elf32.c
+++ b/arch/arm64/kernel/binfmt_elf32.c
@@ -12,6 +12,7 @@
 do {						\
 	clear_thread_flag(TIF_32BIT_AARCH64);	\
 	set_thread_flag(TIF_32BIT);		\
+	set_fs(TASK_SIZE_32);			\
 } while (0)
 
 #define COMPAT_ARCH_DLINFO
diff --git a/arch/arm64/kernel/binfmt_ilp32.c b/arch/arm64/kernel/binfmt_ilp32.c
index a934fd4..a8599c6 100644
--- a/arch/arm64/kernel/binfmt_ilp32.c
+++ b/arch/arm64/kernel/binfmt_ilp32.c
@@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
 do {									\
 	set_thread_flag(TIF_32BIT_AARCH64);				\
 	clear_thread_flag(TIF_32BIT);					\
+	set_fs(TASK_SIZE_32);						\
 } while (0)
 
 #undef ARCH_DLINFO
-- 
2.5.0

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Geert Uytterhoeven]

On Wed, Apr 6, 2016 at 2:29 PM, Yury Norov <ynorov@caviumnetworks.com> wrote:
>> We're already closer to the (future) y2038 than to the (past) introduction of
>> LP64...
>
> This is not about Y2038 at all. In fact, current version doesn't fix
> Y2038 problem, as we decided finally.

Indeed. So these legacy applications have to be fixed later anyway.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Geert Uytterhoeven]

On Wed, Apr 6, 2016 at 2:29 PM, Yury Norov <ynorov@caviumnetworks.com> wrote:
>> We're already closer to the (future) y2038 than to the (past) introduction of
>> LP64...
>
> This is not about Y2038 at all. In fact, current version doesn't fix
> Y2038 problem, as we decided finally.

Indeed. So these legacy applications have to be fixed later anyway.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

Hi Geert,

On Wed, Apr 06, 2016 at 08:51:50AM +0200, Geert Uytterhoeven wrote:
> Hi Yuri,
> 
> On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> > This version is rebased on kernel v4.6-rc2, and has fixes in signal subsystem.
> > It works with updated glibc [1] (though very draft), and tested with LTP.
> >
> > It was tested on QEMU and ThunderX machines. No major difference found.
> > This is RFC because ILP32 is not tested in big-endian mode.
> >
> >  v3: https://lkml.org/lkml/2014/9/3/704
> >  v4: https://lkml.org/lkml/2015/4/13/691
> >  v5: https://lkml.org/lkml/2015/9/29/911
> >
> >  v6:
> >  - time_t, __kenel_off_t and other types turned to be 32-bit
> >    for compatibility reasons (after v5 discussion);
> 
> Reading this sparked my interest, so I went to the links above...

Great! I'll add you to CC than.

> 
> What makes you think these "applications that can’t readily be migrated to LP64
> because they were written assuming an ILP32 data model, and that will never
> become suitable for a LP64 data model and will remain locked into ILP32
> operating environments" are more likely to be fixed for y2038 later, than for
> LP64 now?
> 

It was written by Philipp, not me:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-April/337350.html

I'm not the author of this, and I don't think so. Maybe just because I
didn't see all that legacy nightmare, as Philipp does...

Chris Tyler shares relatively common point of view in his video from
Linaro Connect:
https://www.youtube.com/watch?v=QsVLsw_LrJ0

Briefly, we need it (mostly) for compatibility and (then) for performance.
Maybe Prasun can share more details and examples.

> We're already closer to the (future) y2038 than to the (past) introduction of
> LP64...
> 

This is not about Y2038 at all. In fact, current version doesn't fix
Y2038 problem, as we decided finally.

After v4 and v5, it was spread discussion about what ilp32 should do,
and what not. Finally we decided to be not like aarch32, and not like
lp64, and don't fix any issues specifically, but be standard compat
format, as much as possible. So, any improvements and fixes applied
to generic compat will be applied to ilp32 with minimal efforts.

> These unfixable legacy applications have been spreading through x32 to
> the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
> or is it planned)?

I don't think this is the question you really don't know the answer.
Almost everywhere shiny arm64 comes with old and ugly aarch32 IP core.
If no, like ThunderX, people really worry about that. And for me,
configurable option in kernel sources is better tradeoff than billions
transistors in every chip on market. So Cavium here is more
future-oriented than many others...

The other example is ACPI. We have nice and cute device tree, don't we?
Does it make sense to vendors?

> Lots of resources are spent on maintaining the status quo,
> instead of on fixing the real problems.
> 

I think, compatibility is one of real problems. Aarch32 is hardware
solution, and ilp32 is software one.

Yury.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Geert Uytterhoeven]

Hi Yuri,

On Wed, Apr 6, 2016 at 12:08 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> This version is rebased on kernel v4.6-rc2, and has fixes in signal subsystem.
> It works with updated glibc [1] (though very draft), and tested with LTP.
>
> It was tested on QEMU and ThunderX machines. No major difference found.
> This is RFC because ILP32 is not tested in big-endian mode.
>
>  v3: https://lkml.org/lkml/2014/9/3/704
>  v4: https://lkml.org/lkml/2015/4/13/691
>  v5: https://lkml.org/lkml/2015/9/29/911
>
>  v6:
>  - time_t, __kenel_off_t and other types turned to be 32-bit
>    for compatibility reasons (after v5 discussion);

Reading this sparked my interest, so I went to the links above...

What makes you think these "applications that can’t readily be migrated to LP64
because they were written assuming an ILP32 data model, and that will never
become suitable for a LP64 data model and will remain locked into ILP32
operating environments" are more likely to be fixed for y2038 later, than for
LP64 now?

We're already closer to the (future) y2038 than to the (past) introduction of
LP64...

These unfixable legacy applications have been spreading through x32 to
the shiny new arm64 server architecture (does ppc64el also have an ILP32 mode,
or is it planned)? Lots of resources are spent on maintaining the status quo,
instead of on fixing the real problems.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[RFC6 PATCH v6 00/21] ILP32 for ARM64

[Yury Norov]

This version is rebased on kernel v4.6-rc2, and has fixes in signal subsystem.
It works with updated glibc [1] (though very draft), and tested with LTP.

It was tested on QEMU and ThunderX machines. No major difference found.
This is RFC because ILP32 is not tested in big-endian mode.

 v3: https://lkml.org/lkml/2014/9/3/704
 v4: https://lkml.org/lkml/2015/4/13/691
 v5: https://lkml.org/lkml/2015/9/29/911

 v6:
 - time_t, __kenel_off_t and other types turned to be 32-bit
   for compatibility reasons (after v5 discussion);
 - related changes applied to ILP32 syscall table and handlers;
 - ILP32 VDSO code excluded. It's not mandatory, and caused questions
   during review process. We definitely make sure we will follow up
   with a VDSO later on because it is needed for performance reasons;
 - fixed build issues with different combinations of AARCH32 / ILP32
   enabling in config;
 - ILP32 TLS bug fixed;
 - entry32-common.S introduced to hold wrappers needed for both ILP32
   and AARCH32_EL0;
 - documentation updated according to latest changes;
 - rebased to the current head;
 - coding style re-checked;
 - ILP32 syscall table turned around.

   rfc3:
 - all structures and system calls are just like AARCH32 ones now. with 2
   exceptions: syscalls that take 64-bit parameter in 2 32-bit regosters
   are replaced with LP64 version; struct rt_sigframe is constructed both
   from LP64 and AARCH32 fields to be consistent with AARCH64 register set;
 - documentation rewritten accordingly;
 - common code for all 3 ABIs is moved to separated files for easy use,
   new headers and objects are introduced, incl: is_compat.h, thread_bits.h,
   signal_common.h, signal32_common.h.
 - ILP32 VDSO code restored, Nathans comments are addressed;
 - patch "arm64: ilp32: force IPC_64 in msgctl, shmctl, semctl" removed, as
   Arnd suggested general solution for IPC_64 problem.

   rfc4:
 - sys_ilp32.c syscall list is fixed according to comments;
 - binfmt_elf32.c and binfmt_ilp32.c are introduced to host the code handling
   corresponding formats;
 - statfs64, fstsatfs64 and mmap wrappers are removed;
 - rebased on v4.4-rc8 + http://www.spinics.net/lists/kernel/msg2151759.html

 rfc5:
 - addressed rfc4 comments;
 - turned s390 compat wrappers to be generic and applied it to arm64/ilp32.
   Heiko Carsten and Martin Schwidefsky added to CC as s390 maintainers.

 rfc6:
 - glibc follows new ABI, [1];
 - significant rework for signal subsystem (patches 21, 23) - struct ucontext
   is now corresponds user representation;
 - compat wrappers and 32-bit off_t patchsets are joined with this patchset,
   as for now ilp32 is the only user for them;
 - moved to kernel v4.6-rc2;
 - few minor bugfixes.

[1] https://github.com/norov/glibc/tree/new-api

Andrew Pinski (7):
  arm64: ensure the kernel is compiled for LP64
  arm64: rename COMPAT to AARCH32_EL0 in Kconfig
  arm64: change some CONFIG_COMPAT over to use CONFIG_AARCH32_EL0
    instead
  arm64:uapi: set __BITS_PER_LONG correctly for ILP32 and LP64
  arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use
    it
  arm64: ilp32: introduce ilp32-specific handlers for sigframe and
    ucontext
  arm64:ilp32: add ARM64_ILP32 to Kconfig

Bamvor Jian Zhang (1):
  arm64: compat: change config dependences to aarch32

Philipp Tomsich (1):
  arm64:ilp32: add vdso-ilp32 and use for signal return

Yury Norov (16):
  all: syscall wrappers: add documentation
  all: introduce COMPAT_WRAPPER option and enable it for s390
  all: s390: move wrapper infrastructure to generic headers
  all: s390: move compat_wrappers.c from arch/s390/kernel to kernel/
  all: wrap needed syscalls in generic unistd
  compat ABI: use non-compat openat and open_by_handle_at variants
  32-bit ABI: introduce ARCH_32BIT_OFF_T config option
  arm64: ilp32: add documentation on the ILP32 ABI for ARM64
  thread: move thread bits accessors to separated file
  arm64: introduce is_a32_task and is_a32_thread (for AArch32 compat)
  arm64: ilp32: add is_ilp32_compat_{task,thread} and TIF_32BIT_AARCH64
  arm64: introduce binfmt_elf32.c
  arm64: ilp32: introduce binfmt_ilp32.c
  arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32
  arm64: signal: share lp64 signal routines to ilp32
  arm64: signal32: move ilp32 and aarch32 common code to separated file

 Documentation/adding-syscalls.txt             |  32 +++
 Documentation/arm64/ilp32.txt                 |  13 ++
 arch/Kconfig                                  |   8 +
 arch/arc/Kconfig                              |   1 +
 arch/arm/Kconfig                              |   1 +
 arch/arm64/Kconfig                            |  17 +-
 arch/arm64/Makefile                           |   5 +
 arch/arm64/include/asm/compat.h               |  19 +-
 arch/arm64/include/asm/elf.h                  |  35 +---
 arch/arm64/include/asm/fpsimd.h               |   2 +-
 arch/arm64/include/asm/ftrace.h               |   2 +-
 arch/arm64/include/asm/hwcap.h                |   6 +-
 arch/arm64/include/asm/is_compat.h            |  84 ++++++++
 arch/arm64/include/asm/memory.h               |   3 +-
 arch/arm64/include/asm/processor.h            |  11 +-
 arch/arm64/include/asm/ptrace.h               |   2 +-
 arch/arm64/include/asm/signal32.h             |   6 +-
 arch/arm64/include/asm/signal32_common.h      |  25 +++
 arch/arm64/include/asm/signal_common.h        |  33 +++
 arch/arm64/include/asm/signal_ilp32.h         |  34 ++++
 arch/arm64/include/asm/syscall.h              |   2 +-
 arch/arm64/include/asm/thread_info.h          |   3 +-
 arch/arm64/include/asm/unistd.h               |  11 +-
 arch/arm64/include/asm/unistd32.h             |   2 +-
 arch/arm64/include/asm/vdso.h                 |   6 +
 arch/arm64/include/uapi/asm/bitsperlong.h     |   9 +-
 arch/arm64/kernel/Makefile                    |  12 +-
 arch/arm64/kernel/asm-offsets.c               |   2 +-
 arch/arm64/kernel/binfmt_elf32.c              |  33 +++
 arch/arm64/kernel/binfmt_ilp32.c              |  91 +++++++++
 arch/arm64/kernel/cpufeature.c                |   8 +-
 arch/arm64/kernel/cpuinfo.c                   |   4 +-
 arch/arm64/kernel/entry.S                     |  18 +-
 arch/arm64/kernel/entry_ilp32.S               |  23 +++
 arch/arm64/kernel/head.S                      |   2 +-
 arch/arm64/kernel/hw_breakpoint.c             |  10 +-
 arch/arm64/kernel/perf_regs.c                 |   2 +-
 arch/arm64/kernel/process.c                   |   7 +-
 arch/arm64/kernel/ptrace.c                    |  67 ++++++-
 arch/arm64/kernel/signal.c                    | 100 ++++++----
 arch/arm64/kernel/signal32.c                  |  85 --------
 arch/arm64/kernel/signal32_common.c           | 115 +++++++++++
 arch/arm64/kernel/signal_ilp32.c              | 192 ++++++++++++++++++
 arch/arm64/kernel/sys32.c                     |   1 +
 arch/arm64/kernel/sys_ilp32.c                 |  68 +++++++
 arch/arm64/kernel/traps.c                     |   5 +-
 arch/arm64/kernel/vdso-ilp32/.gitignore       |   2 +
 arch/arm64/kernel/vdso-ilp32/Makefile         |  72 +++++++
 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S     |  33 +++
 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S |  95 +++++++++
 arch/arm64/kernel/vdso.c                      |  65 ++++--
 arch/blackfin/Kconfig                         |   1 +
 arch/cris/Kconfig                             |   1 +
 arch/frv/Kconfig                              |   1 +
 arch/h8300/Kconfig                            |   1 +
 arch/hexagon/Kconfig                          |   1 +
 arch/m32r/Kconfig                             |   1 +
 arch/m68k/Kconfig                             |   1 +
 arch/metag/Kconfig                            |   1 +
 arch/microblaze/Kconfig                       |   1 +
 arch/mips/Kconfig                             |   1 +
 arch/mn10300/Kconfig                          |   1 +
 arch/nios2/Kconfig                            |   1 +
 arch/openrisc/Kconfig                         |   1 +
 arch/parisc/Kconfig                           |   1 +
 arch/powerpc/Kconfig                          |   1 +
 arch/s390/Kconfig                             |   1 +
 arch/s390/include/asm/compat.h                |  17 +-
 arch/s390/kernel/Makefile                     |   2 +-
 arch/s390/kernel/compat_linux.c               |   4 +
 arch/s390/kernel/compat_wrapper.c             | 180 -----------------
 arch/score/Kconfig                            |   1 +
 arch/sh/Kconfig                               |   1 +
 arch/sparc/Kconfig                            |   1 +
 arch/tile/Kconfig                             |   1 +
 arch/tile/kernel/compat.c                     |   3 +
 arch/unicore32/Kconfig                        |   1 +
 arch/x86/Kconfig                              |   1 +
 arch/x86/um/Kconfig                           |   1 +
 arch/xtensa/Kconfig                           |   1 +
 drivers/clocksource/arm_arch_timer.c          |   2 +-
 include/linux/compat.h                        | 277 ++++++++++++++++++++++++++
 include/linux/fcntl.h                         |   2 +-
 include/linux/ptrace.h                        |   6 +
 include/linux/syscalls.h                      |  57 +-----
 include/linux/syscalls_structs.h              |  60 ++++++
 include/linux/thread_bits.h                   |  55 +++++
 include/linux/thread_info.h                   |  44 +---
 include/uapi/asm-generic/unistd.h             | 231 ++++++++++-----------
 kernel/Makefile                               |   1 +
 kernel/compat_wrapper.c                       | 175 ++++++++++++++++
 kernel/ptrace.c                               |  10 +-
 92 files changed, 1997 insertions(+), 637 deletions(-)
 create mode 100644 Documentation/arm64/ilp32.txt
 create mode 100644 arch/arm64/include/asm/is_compat.h
 create mode 100644 arch/arm64/include/asm/signal32_common.h
 create mode 100644 arch/arm64/include/asm/signal_common.h
 create mode 100644 arch/arm64/include/asm/signal_ilp32.h
 create mode 100644 arch/arm64/kernel/binfmt_elf32.c
 create mode 100644 arch/arm64/kernel/binfmt_ilp32.c
 create mode 100644 arch/arm64/kernel/entry_ilp32.S
 create mode 100644 arch/arm64/kernel/signal32_common.c
 create mode 100644 arch/arm64/kernel/signal_ilp32.c
 create mode 100644 arch/arm64/kernel/sys_ilp32.c
 create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
 create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
 create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
 create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
 delete mode 100644 arch/s390/kernel/compat_wrapper.c
 create mode 100644 include/linux/syscalls_structs.h
 create mode 100644 include/linux/thread_bits.h
 create mode 100644 kernel/compat_wrapper.c

-- 
2.5.0