From: Catalin Marinas <catalin.marinas@arm.com>
To: Yury Norov <ynorov@caviumnetworks.com>
Cc: linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
arnd@arndb.de, pinskia@gmail.com,
Prasun.Kapoor@caviumnetworks.com, heiko.carstens@de.ibm.com,
linux-doc@vger.kernel.org, Nathan_Lynch@mentor.com,
linux-kernel@vger.kernel.org, agraf@suse.de,
klimov.linux@gmail.com, broonie@kernel.org,
bamvor.zhangjian@huawei.com,
linux-arm-kernel@lists.infradead.org, schwab@suse.de,
schwidefsky@de.ibm.com, joseph@codesourcery.com,
christoph.muellner@theobroma-systems.com
Subject: Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64
Date: Thu, 12 May 2016 16:28:06 +0100 [thread overview]
Message-ID: <20160512152805.GJ11226@e104818-lin.cambridge.arm.com> (raw)
In-Reply-To: <20160512142457.GC30205@yury-N73SV>
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
next prev parent reply other threads:[~2016-05-12 15:28 UTC|newest]
Thread overview: 135+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-05 22:08 [RFC6 PATCH v6 00/21] ILP32 for ARM64 Yury Norov
2016-04-05 22:08 ` [PATCH 01/25] all: syscall wrappers: add documentation Yury Norov
2016-04-05 22:08 ` [PATCH 02/25] all: introduce COMPAT_WRAPPER option and enable it for s390 Yury Norov
2016-04-05 22:08 ` [PATCH 03/25] all: s390: move wrapper infrastructure to generic headers Yury Norov
2016-04-05 22:08 ` [PATCH 04/25] all: s390: move compat_wrappers.c from arch/s390/kernel to kernel/ Yury Norov
2016-04-05 22:08 ` [PATCH 05/25] all: wrap needed syscalls in generic unistd Yury Norov
2016-04-05 22:08 ` [PATCH 06/25] compat ABI: use non-compat openat and open_by_handle_at variants Yury Norov
2016-04-05 22:08 ` [PATCH 07/25] 32-bit ABI: introduce ARCH_32BIT_OFF_T config option Yury Norov
2016-04-05 22:08 ` [PATCH 08/25] arm64: ilp32: add documentation on the ILP32 ABI for ARM64 Yury Norov
2016-04-05 22:08 ` [PATCH 09/25] arm64: ensure the kernel is compiled for LP64 Yury Norov
2016-04-05 22:08 ` [PATCH 10/25] arm64: rename COMPAT to AARCH32_EL0 in Kconfig Yury Norov
2016-04-14 3:20 ` Zhangjian (Bamvor)
2016-04-22 15:28 ` Catalin Marinas
2016-04-05 22:08 ` [PATCH 11/25] arm64: change some CONFIG_COMPAT over to use CONFIG_AARCH32_EL0 instead Yury Norov
2016-04-05 22:08 ` [PATCH 12/25] arm64: compat: change config dependences to aarch32 Yury Norov
2016-04-22 15:58 ` Catalin Marinas
2016-04-22 21:59 ` Yury Norov
2016-04-23 2:33 ` Zhangjian (Bamvor)
2016-04-05 22:08 ` [PATCH 13/25] arm64:uapi: set __BITS_PER_LONG correctly for ILP32 and LP64 Yury Norov
2016-04-05 22:08 ` [PATCH 14/25] thread: move thread bits accessors to separated file Yury Norov
2016-04-05 22:08 ` [PATCH 15/25] arm64: introduce is_a32_task and is_a32_thread (for AArch32 compat) Yury Norov
2016-05-06 12:02 ` Zhangjian (Bamvor)
2016-04-05 22:08 ` [PATCH 16/25] arm64: ilp32: add is_ilp32_compat_{task,thread} and TIF_32BIT_AARCH64 Yury Norov
2016-04-05 22:08 ` [PATCH 17/25] arm64: introduce binfmt_elf32.c Yury Norov
2016-04-05 22:08 ` [PATCH 18/25] arm64: ilp32: introduce binfmt_ilp32.c Yury Norov
2016-04-22 16:51 ` Catalin Marinas
2016-04-22 17:41 ` Arnd Bergmann
2016-04-05 22:08 ` [PATCH 19/25] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32 Yury Norov
2016-04-22 17:10 ` Catalin Marinas
2016-04-22 21:40 ` Yury Norov
2016-04-25 16:57 ` Catalin Marinas
2016-04-05 22:08 ` [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Yury Norov
2016-04-25 17:26 ` Catalin Marinas
2016-04-25 18:19 ` Yury Norov
2016-04-25 18:47 ` Yury Norov
2016-04-26 10:08 ` Catalin Marinas
2016-04-26 16:57 ` Catalin Marinas
2016-04-28 19:19 ` Yury Norov
2016-04-28 20:43 ` Arnd Bergmann
2016-04-28 22:21 ` Yury Norov
2016-04-28 22:43 ` Arnd Bergmann
2016-04-29 13:13 ` Yury Norov
2016-04-29 15:45 ` Arnd Bergmann
2016-05-06 12:16 ` Zhangjian (Bamvor)
2016-05-06 12:37 ` Yury Norov
2016-05-10 7:42 ` Zhangjian (Bamvor)
2016-05-10 7:55 ` Zhangjian (Bamvor)
2016-05-10 8:36 ` Arnd Bergmann
2016-05-10 9:47 ` Zhangjian (Bamvor)
2016-05-10 11:48 ` Arnd Bergmann
2016-05-10 12:39 ` Zhangjian (Bamvor)
2016-05-10 12:50 ` Arnd Bergmann
2016-05-11 2:04 ` Zhangjian (Bamvor)
2016-05-11 8:04 ` Yury Norov
2016-05-11 10:55 ` Arnd Bergmann
2016-05-11 16:59 ` Catalin Marinas
2016-05-11 19:30 ` Arnd Bergmann
2016-05-12 9:17 ` Catalin Marinas
2016-05-12 9:21 ` Arnd Bergmann
2016-05-12 12:49 ` Zhangjian (Bamvor)
2016-05-12 13:06 ` Arnd Bergmann
2016-05-12 13:19 ` Yury Norov
2016-05-14 12:49 ` Yury Norov
2016-05-11 8:09 ` Arnd Bergmann
2016-05-11 10:12 ` Zhangjian (Bamvor)
2016-05-11 11:16 ` Zhangjian (Bamvor)
2016-05-11 14:50 ` Arnd Bergmann
2016-05-12 3:45 ` Zhangjian (Bamvor)
2016-05-12 8:24 ` Yury Norov
2016-05-12 12:52 ` Zhangjian (Bamvor)
2016-05-12 12:59 ` Yury Norov
2016-05-14 15:03 ` Yury Norov
2016-05-16 17:06 ` Catalin Marinas
2016-05-17 19:05 ` Yury Norov
2016-05-18 11:21 ` Catalin Marinas
2016-05-18 17:58 ` Yury Norov
2016-04-05 22:08 ` [PATCH 21/25] arm64: signal: share lp64 signal routines to ilp32 Yury Norov
2016-04-05 22:08 ` [PATCH 22/25] arm64: signal32: move ilp32 and aarch32 common code to separated file Yury Norov
2016-04-05 22:08 ` [PATCH 23/25] arm64: ilp32: introduce ilp32-specific handlers for sigframe and ucontext Yury Norov
2016-04-05 22:08 ` [PATCH 24/25] arm64:ilp32: add vdso-ilp32 and use for signal return Yury Norov
2016-04-13 9:19 ` Zhangjian (Bamvor)
2016-04-13 15:55 ` Yury Norov
2016-05-03 12:49 ` Zhangjian (Bamvor)
2016-04-29 16:01 ` Catalin Marinas
2016-04-29 17:30 ` Arnd Bergmann
2016-05-03 9:00 ` Catalin Marinas
2016-05-03 9:05 ` Arnd Bergmann
2016-05-03 11:07 ` Zhangjian (Bamvor)
2016-05-03 12:41 ` Zhangjian (Bamvor)
2016-05-04 21:49 ` Yury Norov
2016-05-04 23:23 ` Andrew Pinski
2016-05-05 2:24 ` Zhangjian (Bamvor)
2016-05-05 6:40 ` Andrew Pinski
2016-05-06 14:00 ` Yury Norov
2016-05-09 10:07 ` Zhangjian (Bamvor)
2016-05-05 8:22 ` Yury Norov
2016-04-05 22:08 ` [PATCH 25/25] arm64:ilp32: add ARM64_ILP32 to Kconfig Yury Norov
2016-04-29 16:03 ` Catalin Marinas
2016-04-29 16:08 ` Yury Norov
2016-04-29 16:14 ` Catalin Marinas
2016-04-29 16:26 ` Yury Norov
2016-04-05 22:44 ` [RFC6 PATCH v6 00/21] ILP32 for ARM64 - LTP results Yury Norov
2016-04-23 3:37 ` Zhangjian (Bamvor)
2016-04-27 7:30 ` Andrew Pinski
2016-04-27 21:15 ` Andrew Pinski
2016-04-28 12:16 ` Zhangjian (Bamvor)
2016-04-06 6:51 ` [RFC6 PATCH v6 00/21] ILP32 for ARM64 Geert Uytterhoeven
2016-04-06 12:29 ` Yury Norov
2016-04-07 12:28 ` Geert Uytterhoeven
2016-05-12 0:20 ` Yury Norov
2016-05-12 9:19 ` Arnd Bergmann
2016-05-12 10:30 ` Yury Norov
2016-05-12 13:35 ` Catalin Marinas
2016-05-12 13:44 ` Yury Norov
2016-05-12 14:07 ` Catalin Marinas
2016-05-12 14:20 ` Catalin Marinas
2016-05-12 14:34 ` Yury Norov
2016-05-12 14:54 ` Catalin Marinas
2016-05-12 15:27 ` Yury Norov
2016-05-12 14:24 ` Yury Norov
2016-05-12 15:28 ` Catalin Marinas [this message]
2016-05-13 8:11 ` Zhangjian (Bamvor)
2016-05-13 9:28 ` Catalin Marinas
2016-05-13 10:51 ` Yury Norov
2016-05-13 11:03 ` Catalin Marinas
2016-05-13 13:32 ` Catalin Marinas
2016-05-17 12:10 ` Szabolcs Nagy
2016-05-17 15:37 ` Arnd Bergmann
2016-05-17 15:45 ` Joseph Myers
2016-05-17 16:02 ` Andreas Schwab
2016-05-17 22:45 ` Arnd Bergmann
2016-05-17 15:40 ` Joseph Myers
2016-04-07 12:18 Adam Borowski
2016-04-08 2:49 ` Andrew Pinski
2016-04-09 2:42 ` Arnd Bergmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160512152805.GJ11226@e104818-lin.cambridge.arm.com \
--to=catalin.marinas@arm.com \
--cc=Nathan_Lynch@mentor.com \
--cc=Prasun.Kapoor@caviumnetworks.com \
--cc=agraf@suse.de \
--cc=arnd@arndb.de \
--cc=bamvor.zhangjian@huawei.com \
--cc=broonie@kernel.org \
--cc=christoph.muellner@theobroma-systems.com \
--cc=heiko.carstens@de.ibm.com \
--cc=joseph@codesourcery.com \
--cc=klimov.linux@gmail.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=pinskia@gmail.com \
--cc=schwab@suse.de \
--cc=schwidefsky@de.ibm.com \
--cc=ynorov@caviumnetworks.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).