* Re: [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at nptl init and thread creation (v4)
From: Florian Weimer @ 2019-01-14 18:57 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: carlos, Joseph Myers, Szabolcs Nagy, libc-alpha, Thomas Gleixner,
Ben Maurer, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Will Deacon, Dave Watson, Paul Turner, Rich Felker, linux-kernel,
linux-api
In-Reply-To: <1488546469.1564.1547169116539.JavaMail.zimbra@efficios.com>
* Mathieu Desnoyers:
> Now that I think about it, it's important to move the rseq registration
> done at nptl init (in my current code) to some lower-level csu initialiation,
> so applications that happen _not_ to link against libpthread also get
> registered rseq for the main thread.
Yes. In general, we want to avoid to force libraries which do not
create threads to link against libpthread, and try to provide interfaces
which are required for synchronization within libc.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at nptl init and thread creation (v4)
From: Florian Weimer @ 2019-01-14 18:55 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: carlos, Joseph Myers, Szabolcs Nagy, libc-alpha, Thomas Gleixner,
Ben Maurer, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Will Deacon, Dave Watson, Paul Turner, Rich Felker, linux-kernel,
linux-api
In-Reply-To: <1681283664.1380.1547152315426.JavaMail.zimbra@efficios.com>
* Mathieu Desnoyers:
> Therefore, both symbols will end up in
> sysdeps/unix/sysv/linux/Versions.
I'm not sure what you mean by that. The physical location in the
directory tree has little effect on which shared object the symbol is
placed in; that will need other changes.
>> By the way, you could avoid the need for unregistration if you allocated
>> the rseq areas persistently, index by TID. They are quite small, so
>> with the typical PID range, maybe the wasted memory due to changing TIDs
>> would be acceptable?
>
> Would we be able to access those __rseq_abi as normal TLS IE model
> variables ? The overhead of indexing an array matters for a
> fast-path.
No, that wouldn't be possible in this case. You would need another
indirection.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH 14/15] arch: add split IPC system calls where needed
From: Michael Ellerman @ 2019-01-14 9:58 UTC (permalink / raw)
To: y2038, linux-api, linux-kernel
Cc: Arnd Bergmann, ink, mattst88, linux, catalin.marinas, will.deacon,
tony.luck, fenghua.yu, geert, monstr, paul.burton, deller,
schwidefsky, heiko.carstens, dalias, davem, luto, tglx, mingo,
hpa, x86, jcmvbkbc, firoz.khan, ebiederm, deepa.kernel, linux,
akpm, dave, linux-alpha, linux-arm-kernel, linux-ia64, linux-m68k,
linux-mips, linux-parisc
In-Reply-To: <87pnsz28k2.fsf@concordia.ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> Hi Arnd,
>
> Arnd Bergmann <arnd@arndb.de> writes:
>> The IPC system call handling is highly inconsistent across architectures,
>> some use sys_ipc, some use separate calls, and some use both. We also
>> have some architectures that require passing IPC_64 in the flags, and
>> others that set it implicitly.
...
>
> We already have a gap at 366-377 from when we tried to add the split IPC
> calls a few years back.
>
> I guess I don't mind leaving that gap and using the common numbers as
> you've done here.
>
> But it would be good to add a comment pointing out that we have room
> at 366 for more arch specific syscalls as well.
>
> cheers
Guess I sent that one twice. 🤦
cheers
^ permalink raw reply
* Re: [PATCH 15/15] arch: add pkey and rseq syscall numbers everywhere
From: Heiko Carstens @ 2019-01-14 8:31 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Ivan Kokshaysky, Matt Turner, Russell King - ARM Linux,
Catalin Marinas, Will Deacon, Tony Luck, Fenghua Yu,
Geert Uytterhoeven, Michal Simek, Paul Burton, Helge Deller,
Michael Ellerman, Martin Schwidefsky, Rich Felker, David Miller,
Andy Lutomirski
In-Reply-To: <CAK8P3a0M6qSJQAtFNKVS5muissKvyzeE9MSYT_uwKnM4BKCAug@mail.gmail.com>
On Fri, Jan 11, 2019 at 06:30:43PM +0100, Arnd Bergmann wrote:
> On Thu, Jan 10, 2019 at 9:36 PM Heiko Carstens
> <heiko.carstens@de.ibm.com> wrote:
> > On Thu, Jan 10, 2019 at 05:24:35PM +0100, Arnd Bergmann wrote:
>
> > Since you only need/want the system call numbers, could you please
> > change these lines to:
> >
> > > +384 common pkey_alloc - -
> > > +385 common pkey_free - -
> > > +386 common pkey_mprotect - -
> >
> > Otherwise it _looks_ like we would need compat wrappers here as well,
> > even though all of them would just jump to sys_ni_syscall() in this
> > case. Making this explicit seems to better.
>
> Ok, fair enough. I considered doing this originally and then
> decided against it for consistency with the asm-generic file,
> but I don't care much either way.
>
> Is this something you may want to add later? I'm not sure exactly
> how pkey compares to s390 storage keys, or if this is something
> completely unrelated.
I don't think pkeys will ever work on s390, since they require a key
per mapping, while the s390 storage keys are per physical page.
^ permalink raw reply
* Re: [PATCH 14/15] arch: add split IPC system calls where needed
From: Michael Ellerman @ 2019-01-14 3:59 UTC (permalink / raw)
To: y2038, linux-api, linux-kernel
Cc: Arnd Bergmann, ink, mattst88, linux, catalin.marinas, will.deacon,
tony.luck, fenghua.yu, geert, monstr, paul.burton, deller,
schwidefsky, heiko.carstens, dalias, davem, luto, tglx, mingo,
hpa, x86, jcmvbkbc, firoz.khan, ebiederm, deepa.kernel, linux,
akpm, dave, linux-alpha, linux-arm-kernel, linux-ia64, linux-m68k,
linux-mips, linux-parisc
In-Reply-To: <20190110162435.309262-15-arnd@arndb.de>
Hi Arnd,
Arnd Bergmann <arnd@arndb.de> writes:
> The IPC system call handling is highly inconsistent across architectures,
> some use sys_ipc, some use separate calls, and some use both. We also
> have some architectures that require passing IPC_64 in the flags, and
> others that set it implicitly.
>
> For the additon of a y2083 safe semtimedop() system call, I chose to only
> support the separate entry points, but that requires first supporting
> the regular ones with their own syscall numbers.
>
> The IPC_64 is now implied by the new semctl/shmctl/msgctl system
> calls even on the architectures that require passing it with the ipc()
> multiplexer.
>
> I'm not adding the new semtimedop() or semop() on 32-bit architectures,
> those will get implemented using the new semtimedop_time64() version
> that gets added along with the other time64 calls.
> Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop().
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> One aspect here that might be a bit controversial is the use of
> the same system call numbers across all architectures, synchronizing
> all of them with the x86-32 numbers. With the new syscall.tbl
> files, I hope we can just keep doing that in the future, and no
> longer require the architecture maintainers to assign a number.
>
> This is mainly useful for implementers of the C libraries: if
> we can add future system calls everywhere at the same time, using
> a particular version of the kernel headers also guarantees that
> the system call number macro is visible.
> ---
> arch/m68k/kernel/syscalls/syscall.tbl | 11 +++++++++++
> arch/mips/kernel/syscalls/syscall_o32.tbl | 11 +++++++++++
> arch/powerpc/kernel/syscalls/syscall.tbl | 12 ++++++++++++
I have some changes I'd like to make to our syscall table that will
clash with this.
I'll try and send them today.
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index db3bbb8744af..1bffab54ff35 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -425,3 +425,15 @@
> 386 nospu pkey_mprotect sys_pkey_mprotect
> 387 nospu rseq sys_rseq
> 388 nospu io_pgetevents sys_io_pgetevents compat_sys_io_pgetevents
> +# room for arch specific syscalls
> +392 64 semtimedop sys_semtimedop
> +393 common semget sys_semget
> +394 common semctl sys_semctl compat_sys_semctl
> +395 common shmget sys_shmget
> +396 common shmctl sys_shmctl compat_sys_shmctl
> +397 common shmat sys_shmat compat_sys_shmat
> +398 common shmdt sys_shmdt
> +399 common msgget sys_msgget
> +400 common msgsnd sys_msgsnd compat_sys_msgsnd
> +401 common msgrcv sys_msgrcv compat_sys_msgrcv
> +402 common msgctl sys_msgctl compat_sys_msgctl
We already have a gap at 366-377 from when we tried to add the split IPC
calls a few years back.
I guess I don't mind leaving that gap and using the common numbers as
you've done here.
But it would be good to add a comment pointing out that we have room
at 366 for more arch specific syscalls as well.
cheers
^ permalink raw reply
* Re: [PATCH 14/15] arch: add split IPC system calls where needed
From: Michael Ellerman @ 2019-01-14 3:40 UTC (permalink / raw)
To: y2038, linux-api, linux-kernel
Cc: dalias, linux-ia64, linux-sh, catalin.marinas, will.deacon, linux,
jcmvbkbc, deepa.kernel, hpa, sparclinux, linux-s390, dave, deller,
x86, linux, mingo, geert, firoz.khan, mattst88, fenghua.yu,
Arnd Bergmann, heiko.carstens, linux-m68k, ink, luto, tglx,
linux-arm-kernel, monstr, tony.luck, linux-parisc, linux-mips,
paul.burton, ebiederm, linux-alpha, schwidefsky, akpm,
linuxppc-dev
In-Reply-To: <20190110162435.309262-15-arnd@arndb.de>
Hi Arnd,
Arnd Bergmann <arnd@arndb.de> writes:
> The IPC system call handling is highly inconsistent across architectures,
> some use sys_ipc, some use separate calls, and some use both. We also
> have some architectures that require passing IPC_64 in the flags, and
> others that set it implicitly.
>
> For the additon of a y2083 safe semtimedop() system call, I chose to only
> support the separate entry points, but that requires first supporting
> the regular ones with their own syscall numbers.
>
> The IPC_64 is now implied by the new semctl/shmctl/msgctl system
> calls even on the architectures that require passing it with the ipc()
> multiplexer.
>
> I'm not adding the new semtimedop() or semop() on 32-bit architectures,
> those will get implemented using the new semtimedop_time64() version
> that gets added along with the other time64 calls.
> Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop().
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> One aspect here that might be a bit controversial is the use of
> the same system call numbers across all architectures, synchronizing
> all of them with the x86-32 numbers. With the new syscall.tbl
> files, I hope we can just keep doing that in the future, and no
> longer require the architecture maintainers to assign a number.
>
> This is mainly useful for implementers of the C libraries: if
> we can add future system calls everywhere at the same time, using
> a particular version of the kernel headers also guarantees that
> the system call number macro is visible.
> ---
> arch/m68k/kernel/syscalls/syscall.tbl | 11 +++++++++++
> arch/mips/kernel/syscalls/syscall_o32.tbl | 11 +++++++++++
> arch/powerpc/kernel/syscalls/syscall.tbl | 12 ++++++++++++
I have some changes I'd like to make to our syscall table that will
clash with this.
I'll try and send them today.
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index db3bbb8744af..1bffab54ff35 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -425,3 +425,15 @@
> 386 nospu pkey_mprotect sys_pkey_mprotect
> 387 nospu rseq sys_rseq
> 388 nospu io_pgetevents sys_io_pgetevents compat_sys_io_pgetevents
> +# room for arch specific syscalls
> +392 64 semtimedop sys_semtimedop
> +393 common semget sys_semget
> +394 common semctl sys_semctl compat_sys_semctl
> +395 common shmget sys_shmget
> +396 common shmctl sys_shmctl compat_sys_shmctl
> +397 common shmat sys_shmat compat_sys_shmat
> +398 common shmdt sys_shmdt
> +399 common msgget sys_msgget
> +400 common msgsnd sys_msgsnd compat_sys_msgsnd
> +401 common msgrcv sys_msgrcv compat_sys_msgrcv
> +402 common msgctl sys_msgctl compat_sys_msgctl
We already have a gap at 366-377 from when we tried to add the split IPC
calls a few years back.
I guess I don't mind leaving that gap and using the common numbers.
But would be good to add a comment pointing out that we have room there
for arch specific syscalls as well.
cheers
^ permalink raw reply
* [PATCH -manpage 2/2] memfd_create.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
From: Joel Fernandes @ 2019-01-13 0:14 UTC (permalink / raw)
To: linux-kernel, mtk.manpages
Cc: Joel Fernandes (Google), Andrew Morton, Andy Lutomirski, dancol,
Hugh Dickins, Jann Horn, John Stultz, linux-api, linux-man,
linux-mm, marcandre.lureau, Matthew Wilcox, Mike Kravetz,
Shuah Khan, Stephen Rothwell
In-Reply-To: <20190113001446.158789-1-joel@joelfernandes.org>
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
More details of the seal can be found in the LKML patch:
https://lore.kernel.org/lkml/20181120052137.74317-1-joel@joelfernandes.org/T/#t
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
man2/memfd_create.2 | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/man2/memfd_create.2 b/man2/memfd_create.2
index 3cd392d1b..fce2bf8d0 100644
--- a/man2/memfd_create.2
+++ b/man2/memfd_create.2
@@ -280,7 +280,15 @@ in order to restrict further modifications on the file.
(If placing the seal
.BR F_SEAL_WRITE ,
then it will be necessary to first unmap the shared writable mapping
-created in the previous step.)
+created in the previous step. Otherwise, behavior similar to
+.BR F_SEAL_WRITE
+can be achieved, by using
+.BR F_SEAL_FUTURE_WRITE
+which will prevent future writes via
+.BR mmap (2)
+and
+.BR write (2)
+from succeeding, while keeping existing shared writable mappings).
.IP 4.
A second process obtains a file descriptor for the
.BR tmpfs (5)
@@ -425,6 +433,7 @@ main(int argc, char *argv[])
fprintf(stderr, "\\t\\tg \- F_SEAL_GROW\\n");
fprintf(stderr, "\\t\\ts \- F_SEAL_SHRINK\\n");
fprintf(stderr, "\\t\\tw \- F_SEAL_WRITE\\n");
+ fprintf(stderr, "\\t\\tW \- F_SEAL_FUTURE_WRITE\\n");
fprintf(stderr, "\\t\\tS \- F_SEAL_SEAL\\n");
exit(EXIT_FAILURE);
}
@@ -463,6 +472,8 @@ main(int argc, char *argv[])
seals |= F_SEAL_SHRINK;
if (strchr(seals_arg, \(aqw\(aq) != NULL)
seals |= F_SEAL_WRITE;
+ if (strchr(seals_arg, \(aqW\(aq) != NULL)
+ seals |= F_SEAL_FUTURE_WRITE;
if (strchr(seals_arg, \(aqS\(aq) != NULL)
seals |= F_SEAL_SEAL;
@@ -518,6 +529,8 @@ main(int argc, char *argv[])
printf(" GROW");
if (seals & F_SEAL_WRITE)
printf(" WRITE");
+ if (seals & F_SEAL_FUTURE_WRITE)
+ printf(" FUTURE_WRITE");
if (seals & F_SEAL_SHRINK)
printf(" SHRINK");
printf("\\n");
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH -manpage 1/2] fcntl.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
From: Joel Fernandes @ 2019-01-13 0:14 UTC (permalink / raw)
To: linux-kernel, mtk.manpages
Cc: Joel Fernandes (Google), Andrew Morton, Andy Lutomirski, dancol,
Hugh Dickins, Jann Horn, John Stultz, linux-api, linux-man,
linux-mm, marcandre.lureau, Matthew Wilcox, Mike Kravetz,
Shuah Khan, Stephen Rothwell
In-Reply-To: <20190113001446.158789-1-joel@joelfernandes.org>
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
More details of the seal can be found in the LKML patch:
https://lore.kernel.org/lkml/20181120052137.74317-1-joel@joelfernandes.org/T/#t
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
man2/fcntl.2 | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/man2/fcntl.2 b/man2/fcntl.2
index 03533d65b..54772f949 100644
--- a/man2/fcntl.2
+++ b/man2/fcntl.2
@@ -1525,6 +1525,21 @@ Furthermore, if there are any asynchronous I/O operations
.RB ( io_submit (2))
pending on the file,
all outstanding writes will be discarded.
+.TP
+.BR F_SEAL_FUTURE_WRITE
+If this seal is set, the contents of the file can be modified only from
+existing writeable mappings that were created prior to the seal being set.
+Any attempt to create a new writeable mapping on the memfd via
+.BR mmap (2)
+will fail with
+.BR EPERM.
+Also any attempts to write to the memfd via
+.BR write (2)
+will fail with
+.BR EPERM.
+This is useful in situations where existing writable mapped regions need to be
+kept intact while preventing any future writes. For example, to share a
+read-only memory buffer to other processes that only the sender can write to.
.\"
.SS File read/write hints
Write lifetime hints can be used to inform the kernel about the relative
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH -manpage 0/2] Document memfd F_SEAL_FUTURE_WRITE seal
From: Joel Fernandes @ 2019-01-13 0:14 UTC (permalink / raw)
To: linux-kernel, mtk.manpages
Cc: Joel Fernandes, Andrew Morton, Andy Lutomirski, dancol,
Hugh Dickins, Jann Horn, John Stultz, linux-api, linux-man,
linux-mm, marcandre.lureau, Matthew Wilcox, Mike Kravetz,
Shuah Khan, Stephen Rothwell
Hello,
These manpages correspond to the following kernel patches:
https://lore.kernel.org/patchwork/patch/1031550/
https://lore.kernel.org/patchwork/patch/1031551/
This is just a resend with no changes from last time.
Joel Fernandes (Google) (2):
fcntl.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
memfd_create.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
man2/fcntl.2 | 15 +++++++++++++++
man2/memfd_create.2 | 15 ++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply
* [PATCH v4 2/2] selftests/memfd: Add tests for F_SEAL_FUTURE_WRITE seal
From: Joel Fernandes @ 2019-01-12 20:38 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), dancol, minchan, Jann Horn, John Stultz,
Al Viro, Andrew Morton, Andy Lutomirski, Hugh Dickins,
J. Bruce Fields, Jeff Layton, linux-api, linux-fsdevel,
linux-kselftest, linux-mm, Marc-André Lureau, Matthew Wilcox,
Mike Kravetz, Shuah Khan
In-Reply-To: <20190112203816.85534-1-joel@joelfernandes.org>
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Add tests to verify sealing memfds with the F_SEAL_FUTURE_WRITE works as
expected.
Cc: dancol@google.com
Cc: minchan@kernel.org
Cc: Jann Horn <jannh@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
tools/testing/selftests/memfd/memfd_test.c | 74 ++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
index 10baa1652fc2..c67d32eeb668 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -54,6 +54,22 @@ static int mfd_assert_new(const char *name, loff_t sz, unsigned int flags)
return fd;
}
+static int mfd_assert_reopen_fd(int fd_in)
+{
+ int r, fd;
+ char path[100];
+
+ sprintf(path, "/proc/self/fd/%d", fd_in);
+
+ fd = open(path, O_RDWR);
+ if (fd < 0) {
+ printf("re-open of existing fd %d failed\n", fd_in);
+ abort();
+ }
+
+ return fd;
+}
+
static void mfd_fail_new(const char *name, unsigned int flags)
{
int r;
@@ -255,6 +271,25 @@ static void mfd_assert_read(int fd)
munmap(p, mfd_def_size);
}
+/* Test that PROT_READ + MAP_SHARED mappings work. */
+static void mfd_assert_read_shared(int fd)
+{
+ void *p;
+
+ /* verify PROT_READ and MAP_SHARED *is* allowed */
+ p = mmap(NULL,
+ mfd_def_size,
+ PROT_READ,
+ MAP_SHARED,
+ fd,
+ 0);
+ if (p == MAP_FAILED) {
+ printf("mmap() failed: %m\n");
+ abort();
+ }
+ munmap(p, mfd_def_size);
+}
+
static void mfd_assert_write(int fd)
{
ssize_t l;
@@ -692,6 +727,44 @@ static void test_seal_write(void)
close(fd);
}
+/*
+ * Test SEAL_FUTURE_WRITE
+ * Test whether SEAL_FUTURE_WRITE actually prevents modifications.
+ */
+static void test_seal_future_write(void)
+{
+ int fd, fd2;
+ void *p;
+
+ printf("%s SEAL-FUTURE-WRITE\n", memfd_str);
+
+ fd = mfd_assert_new("kern_memfd_seal_future_write",
+ mfd_def_size,
+ MFD_CLOEXEC | MFD_ALLOW_SEALING);
+
+ p = mfd_assert_mmap_shared(fd);
+
+ mfd_assert_has_seals(fd, 0);
+
+ mfd_assert_add_seals(fd, F_SEAL_FUTURE_WRITE);
+ mfd_assert_has_seals(fd, F_SEAL_FUTURE_WRITE);
+
+ /* read should pass, writes should fail */
+ mfd_assert_read(fd);
+ mfd_assert_read_shared(fd);
+ mfd_fail_write(fd);
+
+ fd2 = mfd_assert_reopen_fd(fd);
+ /* read should pass, writes should still fail */
+ mfd_assert_read(fd2);
+ mfd_assert_read_shared(fd2);
+ mfd_fail_write(fd2);
+
+ munmap(p, mfd_def_size);
+ close(fd2);
+ close(fd);
+}
+
/*
* Test SEAL_SHRINK
* Test whether SEAL_SHRINK actually prevents shrinking
@@ -945,6 +1018,7 @@ int main(int argc, char **argv)
test_basic();
test_seal_write();
+ test_seal_future_write();
test_seal_shrink();
test_seal_grow();
test_seal_resize();
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v4 1/2] mm/memfd: Add an F_SEAL_FUTURE_WRITE seal to memfd
From: Joel Fernandes @ 2019-01-12 20:38 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Andy Lutomirski, Al Viro, Andrew Morton,
dancol, Hugh Dickins, Jann Horn, J. Bruce Fields, Jeff Layton,
John Stultz, linux-api, linux-fsdevel, linux-kselftest, linux-mm,
Marc-André Lureau, Matthew Wilcox, Mike Kravetz, minchan,
Shuah Khan
In-Reply-To: <20190112203816.85534-1-joel@joelfernandes.org>
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Android uses ashmem for sharing memory regions. We are looking forward to
migrating all usecases of ashmem to memfd so that we can possibly remove
the ashmem driver in the future from staging while also benefiting from
using memfd and contributing to it. Note staging drivers are also not ABI
and generally can be removed at anytime.
One of the main usecases Android has is the ability to create a region and
mmap it as writeable, then add protection against making any "future"
writes while keeping the existing already mmap'ed writeable-region active.
This allows us to implement a usecase where receivers of the shared
memory buffer can get a read-only view, while the sender continues to
write to the buffer. See CursorWindow documentation in Android for more
details:
https://developer.android.com/reference/android/database/CursorWindow
This usecase cannot be implemented with the existing F_SEAL_WRITE seal.
To support the usecase, this patch adds a new F_SEAL_FUTURE_WRITE seal
which prevents any future mmap and write syscalls from succeeding while
keeping the existing mmap active.
A better way to do F_SEAL_FUTURE_WRITE seal was discussed [1] last week
where we don't need to modify core VFS structures to get the same
behavior of the seal. This solves several side-effects pointed by Andy.
self-tests are provided in later patch to verify the expected semantics.
[1] https://lore.kernel.org/lkml/20181111173650.GA256781@google.com/
[Thanks a lot to Andy for suggestions to improve code]
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
fs/hugetlbfs/inode.c | 2 +-
include/uapi/linux/fcntl.h | 1 +
mm/memfd.c | 3 ++-
mm/shmem.c | 25 ++++++++++++++++++++++---
4 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 53ea3cef526e..3daf471bbd92 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -557,7 +557,7 @@ static long hugetlbfs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
inode_lock(inode);
/* protected by i_mutex */
- if (info->seals & F_SEAL_WRITE) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
inode_unlock(inode);
return -EPERM;
}
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 6448cdd9a350..a2f8658f1c55 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -41,6 +41,7 @@
#define F_SEAL_SHRINK 0x0002 /* prevent file from shrinking */
#define F_SEAL_GROW 0x0004 /* prevent file from growing */
#define F_SEAL_WRITE 0x0008 /* prevent writes */
+#define F_SEAL_FUTURE_WRITE 0x0010 /* prevent future writes while mapped */
/* (1U << 31) is reserved for signed error codes */
/*
diff --git a/mm/memfd.c b/mm/memfd.c
index 97264c79d2cd..650e65a46b9c 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -131,7 +131,8 @@ static unsigned int *memfd_file_seals_ptr(struct file *file)
#define F_ALL_SEALS (F_SEAL_SEAL | \
F_SEAL_SHRINK | \
F_SEAL_GROW | \
- F_SEAL_WRITE)
+ F_SEAL_WRITE | \
+ F_SEAL_FUTURE_WRITE)
static int memfd_add_seals(struct file *file, unsigned int seals)
{
diff --git a/mm/shmem.c b/mm/shmem.c
index 6ece1e2fe76e..3c98cc9655b4 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2125,6 +2125,24 @@ int shmem_lock(struct file *file, int lock, struct user_struct *user)
static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
{
+ struct shmem_inode_info *info = SHMEM_I(file_inode(file));
+
+ if (info->seals & F_SEAL_FUTURE_WRITE) {
+ /*
+ * New PROT_WRITE and MAP_SHARED mmaps are not allowed when
+ * "future write" seal active.
+ */
+ if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE))
+ return -EPERM;
+
+ /*
+ * Since the F_SEAL_FUTURE_WRITE seals allow for a MAP_SHARED
+ * read-only mapping, take care to not allow mprotect to revert
+ * protections.
+ */
+ vma->vm_flags &= ~(VM_MAYWRITE);
+ }
+
file_accessed(file);
vma->vm_ops = &shmem_vm_ops;
if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
@@ -2375,8 +2393,9 @@ shmem_write_begin(struct file *file, struct address_space *mapping,
pgoff_t index = pos >> PAGE_SHIFT;
/* i_mutex is held by caller */
- if (unlikely(info->seals & (F_SEAL_WRITE | F_SEAL_GROW))) {
- if (info->seals & F_SEAL_WRITE)
+ if (unlikely(info->seals & (F_SEAL_GROW |
+ F_SEAL_WRITE | F_SEAL_FUTURE_WRITE))) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE))
return -EPERM;
if ((info->seals & F_SEAL_GROW) && pos + len > inode->i_size)
return -EPERM;
@@ -2639,7 +2658,7 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset,
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(shmem_falloc_waitq);
/* protected by i_mutex */
- if (info->seals & F_SEAL_WRITE) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
error = -EPERM;
goto out;
}
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v4 0/2] Add a future write seal to memfd
From: Joel Fernandes @ 2019-01-12 20:38 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Al Viro, Andrew Morton, Andy Lutomirski,
dancol, Hugh Dickins, Jann Horn, J. Bruce Fields, Jeff Layton,
John Stultz, linux-api, linux-fsdevel, linux-kselftest, linux-mm,
Marc-André Lureau, Matthew Wilcox, Mike Kravetz, minchan,
Shuah Khan
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
This is just a resend of the previous series at
https://lore.kernel.org/patchwork/patch/1014892/
with a small if block refactor as Andy suggested:
https://lore.kernel.org/patchwork/comment/1198679/
All,
Could you please provide your Reviewed-by / Acked-by tags?
I will also resend the manpage changes shortly.
Joel Fernandes (Google) (2):
mm/memfd: Add an F_SEAL_FUTURE_WRITE seal to memfd
selftests/memfd: Add tests for F_SEAL_FUTURE_WRITE seal
fs/hugetlbfs/inode.c | 2 +-
include/uapi/linux/fcntl.h | 1 +
mm/memfd.c | 3 +-
mm/shmem.c | 25 +++++++-
tools/testing/selftests/memfd/memfd_test.c | 74 ++++++++++++++++++++++
5 files changed, 100 insertions(+), 5 deletions(-)
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply
* Re: [PATCH 11/15] mips: fix n32 compat_ipc_parse_version
From: Paul Burton @ 2019-01-11 19:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Paul Burton, dalias@libc.org, linux-ia64@vger.kernel.org,
linux-sh@vger.kernel.org, catalin.marinas@arm.com,
will.deacon@arm.com, linux@dominikbrodowski.net,
linux-mips@vger.kernel.org, jcmvbkbc@gmail.com,
deepa.kernel@gmail.com, hpa@zytor.com, sparclinux@vger.kernel.org,
linux-s390@vger.kernel.org, dave@stgolabs.net,
y2038@lists.linaro.org, mpe@ellerman.id.au, deller@gmx.de,
x86@kernel.org
In-Reply-To: <20190110162435.309262-12-arnd@arndb.de>
Hello,
Arnd Bergmann wrote:
> While reading through the sysvipc implementation, I noticed that the n32
> semctl/shmctl/msgctl system calls behave differently based on whether
> o32 support is enabled or not: Without o32, the IPC_64 flag passed by
> user space is rejected but calls without that flag get IPC_64 behavior.
>
> As far as I can tell, this was inadvertently changed by a cleanup patch
> but never noticed by anyone, possibly nobody has tried using sysvipc
> on n32 after linux-3.19.
>
> Change it back to the old behavior now.
>
> Fixes: 78aaf956ba3a ("MIPS: Compat: Fix build error if CONFIG_MIPS32_COMPAT but no compat ABI.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied to mips-fixes.
Thanks,
Paul
[ This message was auto-generated; if you believe anything is incorrect
then please email paul.burton@mips.com to report it. ]
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH 14/15] arch: add split IPC system calls where needed
From: Arnd Bergmann @ 2019-01-11 17:33 UTC (permalink / raw)
To: Heiko Carstens
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Ivan Kokshaysky, Matt Turner, Russell King - ARM Linux,
Catalin Marinas, Will Deacon, Tony Luck, Fenghua Yu,
Geert Uytterhoeven, Michal Simek, Paul Burton, Helge Deller,
Michael Ellerman, Martin Schwidefsky, Rich Felker, David Miller,
Andy Lutomirski
In-Reply-To: <20190110203253.GA3676@osiris>
On Thu, Jan 10, 2019 at 9:33 PM Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
> On Thu, Jan 10, 2019 at 05:24:34PM +0100, Arnd Bergmann wrote:
> > diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> > index 022fc099b628..428cf512a757 100644
> > --- a/arch/s390/kernel/syscalls/syscall.tbl
> > +++ b/arch/s390/kernel/syscalls/syscall.tbl
> > @@ -391,3 +391,15 @@
> > 381 common kexec_file_load sys_kexec_file_load compat_sys_kexec_file_load
> > 382 common io_pgetevents sys_io_pgetevents compat_sys_io_pgetevents
> > 383 common rseq sys_rseq compat_sys_rseq
> > +# room for arch specific syscalls
> > +392 64 semtimedop sys_semtimedop -
> > +393 common semget sys_semget sys_semget
> ...
> > +395 common shmget sys_shmget sys_shmget
> ...
> > +398 common shmdt sys_shmdt sys_shmdt
> > +399 common msgget sys_msgget sys_msgget
>
> These four need compat system call wrappers, unfortunately... (well,
> actually only shmget and shmdt require them, but let's add them for
> all four). See arch/s390/kernel/compat_wrapper.c
>
> I'm afraid this compat special handling will be even more annoying in
> the future, since s390 will be the only architecture which requires
> this special handling.
>
> _Maybe_ it would make sense to automatically generate a weak compat
> system call wrapper for s390 with the SYSCALL_DEFINE macros, but that
> probably won't work in all cases.
For some reason I was under the impression that s390 already did that.
However, it seems that x86 does, so I'll try to convert the x86 version
for s390, and see if I can get rid of all the wrappers that way.
It would certainly be safer to have the wrappers always present,
especially if we expect future system calls to be added to the
s390 table by whoever implements the syscall itself.
Arnd
^ permalink raw reply
* Re: [PATCH 15/15] arch: add pkey and rseq syscall numbers everywhere
From: Arnd Bergmann @ 2019-01-11 17:30 UTC (permalink / raw)
To: Heiko Carstens
Cc: y2038 Mailman List, Linux API, Linux Kernel Mailing List,
Ivan Kokshaysky, Matt Turner, Russell King - ARM Linux,
Catalin Marinas, Will Deacon, Tony Luck, Fenghua Yu,
Geert Uytterhoeven, Michal Simek, Paul Burton, Helge Deller,
Michael Ellerman, Martin Schwidefsky, Rich Felker, David Miller,
Andy Lutomirski
In-Reply-To: <20190110203638.GB3676@osiris>
On Thu, Jan 10, 2019 at 9:36 PM Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
> On Thu, Jan 10, 2019 at 05:24:35PM +0100, Arnd Bergmann wrote:
> Since you only need/want the system call numbers, could you please
> change these lines to:
>
> > +384 common pkey_alloc - -
> > +385 common pkey_free - -
> > +386 common pkey_mprotect - -
>
> Otherwise it _looks_ like we would need compat wrappers here as well,
> even though all of them would just jump to sys_ni_syscall() in this
> case. Making this explicit seems to better.
Ok, fair enough. I considered doing this originally and then
decided against it for consistency with the asm-generic file,
but I don't care much either way.
Is this something you may want to add later? I'm not sure exactly
how pkey compares to s390 storage keys, or if this is something
completely unrelated.
Arnd
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-11 16:26 UTC (permalink / raw)
To: Dave Chinner
Cc: Dominique Martinet, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <20190111073606.GP27534@dastard>
On Thu, Jan 10, 2019 at 11:36 PM Dave Chinner <david@fromorbit.com> wrote:
>
> > It's only that single page that *matters*. That's the page that the
> > probe reveals the status of - but it's also the page that the probe
> > then *changes* the status of.
>
> It changes the state of it /after/ we've already got the information
> we need from it. It's not up to date, it has to come from disk, we
> return EAGAIN, which means it was not in the cache.
Oh, I see the confusion.
Yes, you get the information about whether something was in the cache
or not, so the side channel does exist to some degree.
But it's actually hugely reduced for a rather important reason: the
_primary_ reason for needing to know whether some page is in the cache
or not is not actually to see if it was ever accessed - it's to see
that the cache has been scrubbed (and to _guide_ the scrubbing), and
*when* it was accessed.
Think of it this way: the buffer cache residency is actually a
horribly bad signal on its own mainly because you generally have a
very high hit-rate. In most normal non-streaming situations with
sufficient amounts of memory you have pretty much everything cached.
So in order to use it as a signal, first you have to first scrub the
cache (because if the page was already there, there's no signal at
all), and then for the signal to be as useful as possible, you're also
going to want to try to get out more than one bit of information: you
are going to try to see the patterns and the timings of how it gets
filled.
And that's actually quite painful. You don't know the initial cache
state, and you're not (in general) controlling the machine entirely,
because there's also that actual other entity that you're trying to
attack and see what it does.
So what you want to do is basically to first make sure the cache is
scrubbed (only for the pages you're interested in!), then trigger
whatever behavior you are looking for, and then look how that affected
the cache.
In other words, you want *multiple* residency status check - first to
see what the cache state is (because you're going to want that for
scrubbing), then to see that "yes, it's gone" when doing the
scrubbing, and then to see the *pattern* and timings of how things are
brought in.
And then you're likely to want to do this over and over again, so that
you can get real data out of the signal.
This is why something that doesn't perturb what you measure is really
important. If the act of measurement brings the page in, then you
can't use it for that "did I successfully scrub it" phase at all, and
you can't use it for measurement but once, so your view into patterns
and timings is going to be *much* worse.
And notice that this is true even if the act of measurement only
affects the *one* page you're measuring. Sure, any additional noise
around it would likely be annoying too, but it's not really necessary
to make the attack much harder to carry out. In fact, it's almost
irrelevant, since the signal you're trying to *see* is going to be
affected by prefetching etc too, so the patterns and timings you need
to look at are in bigger chunks than the readahead thing.
So yes, you as an attacker can remove the prefetching from *your*
load, but you can't remove it from the target load anyway, so you'll
just have to live with it.
Can you brute-force scrubbing? Yes. For something like an L1 cache,
that's easy (well, QoS domains make it harder). For something like a
disk cache, it's much harder, and makes any attempt to read out state
a lot slower. The paper that started this all uses mincore() not just
to see "is the page now scrubbed", but also to guide the scrubbing
itself (working set estimation etc).
And note that in many ways, the *scrubbing* is really the harder part.
Populating the cache is really easy: just read the data you want to
populate.
So if you are looking for a particular signal, say "did this error
case trigger so that it faulted in *that* piece of information", you'd
want to scrub the target, populate everything else, and then try to
measure at "did I trigger that target". Except you wouldn't want to do
it one page at a time but see as much pattern of "they were touched in
this order" as you can, and you'd like to get timing information of
how the pages you are interested were populated too.
And you'd generally do this over and over and over again because
you're trying to read out some signal.
Notice what the expensive operation was? It's the scrubbing.The "did
the target do IO" you might actually even see other ways for the
trivial cases, like even just look at iostat: just pre-populate
everything but the part you care about, then try to trigger whatever
you're searching for, and see if it caused IO or not.
So it's a bit like a chalkboard: in order to read out the result, you
need to erase it first, and doing that blindly is nasty. And you want
to look at timings, which is also really nasty if every time you look,
you smudge the very place you looked at. It makes it hard to see what
somebody else is writing on the board if you're always overwriting
what you just looked at. Did you get some new information? If not, now
you have to go back and do that scrubbing again, and you'll likely be
missing what *else* the person wrote.
Ans as always: there is no "black and white". There is no "absolute
security", and similarly, there is no "absolute leak proof". It's all
about making it inconvenient enough that it's not really practical.
Linus
^ permalink raw reply
* Re: [RESEND PATCH v3 2/2] sysctl: handle overflow for file-max
From: Christian Brauner @ 2019-01-11 14:51 UTC (permalink / raw)
To: Dominik Brodowski, akpm
Cc: keescook, linux-kernel, ebiederm, mcgrof, joe.lawrence, longman,
viro, adobriyan, linux-api
In-Reply-To: <20190110145559.relfx37ocq5xu4by@isilmar-4.linta.de>
On Thu, Jan 10, 2019 at 03:55:59PM +0100, Dominik Brodowski wrote:
> On Thu, Jan 10, 2019 at 03:50:05PM +0100, Christian Brauner wrote:
> > On Tue, Jan 08, 2019 at 08:01:10AM +0100, Dominik Brodowski wrote:
> > > On Mon, Jan 07, 2019 at 11:27:00PM +0100, Christian Brauner wrote:
> > > > @@ -2833,6 +2836,10 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
> > > > break;
> > > > if (neg)
> > > > continue;
> > > > + if ((max && val > *max) || (min && val < *min)) {
> > > > + err = -EINVAL;
> > > > + break;
> > > > + }
> > > > val = convmul * val / convdiv;
> > > > if ((min && val < *min) || (max && val > *max))
> > > > continue;
> > >
> > > This is a generic change which affects all users of
> > > do_proc_doulongvec_minmax() that have extra1 or extra2 set. In sysctl.c, I
> > > do not see another user of proc_doulongvec_minmax() that has extra1 or
> > > extra2 set. However, have you verified whether your patch changes the
> > > behaviour for other files that make use of proc_doulongvec_minmax() or
> > > proc_doulongvec_ms_jiffies_minmax(), and not only of the file-max sysctl?
> >
> > Sorry for the delayed reply. I did look at the callers. The functions
> > that are of interest afaict are:
> >
> > proc_doulongvec_ms_jiffies_minmax
> > proc_doulongvec_minmax
> >
> > So this could be visible when users write values that would overflow the
> > type used in the kernel.
> >
> > I guess your point is whether we are venturing into userspace break
> > territory. Hm... We should probably make sure that we're not regressing
> > anyone else! What do you think if instead of the above patch we did:
>
> Hm, I prefer the original patch -- as the same (valid) reasons which apply
> for the file-max sysctl might also apply to other users of this function
> where extra1 and/or2 extra2 are set.
>
> If there are no other users of this function where extra1 or extra2 are set,
> just add a comment in the commit message:
>
> While this changes the behaviour of __do_proc_doulongvec_minmax(),
> no other existing users in the kernel are affected by this change.
>
> If there are other users of this function where extra1 or extra2 are set,
> you would need to generalize the commit message overall.
Andrew, can you please drop this patch
[RESEND PATCH v3 2/2] sysctl: handle overflow for file-max
from your tree (It should be located at [1] from what I can gather.).
I'll resend it based on Dominik's observation and will generalize the
commit message and also error out *after* the conversion has been done
and not before.
The first patch 1/2 is correct and can be kept.
Thanks!
Christian
[1]: https://www.ozlabs.org/~akpm/mmots/broken-out/sysctl-handle-overflow-for-file-max.patch
^ permalink raw reply
* Re: [PATCH 00/15] arch: synchronize syscall tables in preparation for y2038
From: Geert Uytterhoeven @ 2019-01-11 8:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rich Felker, linux-ia64@vger.kernel.org, Linux-sh list,
Catalin Marinas, Heiko Carstens, Dominik Brodowski, linux-mips,
Max Filippov, Deepa Dinamani, H. Peter Anvin, sparclinux,
linux-s390, Davidlohr Bueso, y2038 Mailman List, Michael Ellerman,
Helge Deller, the arch/x86 maintainers, Russell King, Ingo Molnar,
Firoz Khan, Matt Turner, Fenghua Yu, Will Deacon
In-Reply-To: <CAK8P3a07yNiadLCJcA0Tyfo90YeQ0P2XF-wOEy9XAb8cDTFObw@mail.gmail.com>
Hi Arnd,
On Thu, Jan 10, 2019 at 11:43 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jan 10, 2019 at 7:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Jan 10, 2019 at 6:06 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thu, Jan 10, 2019 at 5:59 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Thu, Jan 10, 2019 at 5:26 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > The system call tables have diverged a bit over the years, and a number
> > > > > of the recent additions never made it into all architectures, for one
> > > > > reason or another.
> > > > >
> > > > > This is an attempt to clean it up as far as we can without breaking
> > > > > compatibility, doing a number of steps:
> > > >
> > > > Thanks a lot!
> > > >
> > > > > - Add system calls that have not yet been integrated into all
> > > > > architectures but that we definitely want there.
> > > >
> > > > It looks like you missed wiring up io_pgetevents() on m68k.
> > > > Is that intentional?
> > >
> > > Yes, I thought I had described that somewhere but maybe I
> > > forgot: semtimedop() and io_pgetevents() get replaced with
> > > time64 versions in the follow-up, so I only added them in
> > > 64-bit architectures. If you think we should have both
> > > io_pgetevents() and io_pgetevents_time32() on all 32-bit
> > > architectures, I can add that as well.
> >
> > Thanks, sounds fine to me.
>
> Just to be sure, you mean it's fine to not add it, not that we should
> add it?
I'm fine with not having the legacy 32-bit ones.
Sorry for being unclear.
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
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Jiri Kosina @ 2019-01-11 7:36 UTC (permalink / raw)
To: Dave Chinner
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <20190110004424.GH27534@dastard>
On Thu, 10 Jan 2019, Dave Chinner wrote:
> Sounds nice from a theoretical POV, but reality has taught us very
> different lessons.
>
> FWIW, a quick check of XFS's history so you understand how long this
> behaviour has been around. It was introduced in the linux port in 2001
> as direct IO support was being added:
>
> commit e837eac23662afae603aaaef7c94bc839c1b8f67
> Author: Steve Lord <lord@sgi.com>
> Date: Mon Mar 5 16:47:52 2001 +0000
>
> Add bounds checking for direct I/O, do the cache invalidation for
> data coherency on direct I/O.
Out of curiosity, which repository is this from please? Even google
doesn't seem to know about this SHA.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dave Chinner @ 2019-01-11 7:36 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dominique Martinet, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <CAHk-=wh-kegfnPC_dmw0A72Sdk4B9tvce-cOR=jEfHDU1-4Eew@mail.gmail.com>
On Thu, Jan 10, 2019 at 11:08:07PM -0800, Linus Torvalds wrote:
> On Thu, Jan 10, 2019 at 8:04 PM Dave Chinner <david@fromorbit.com> wrote:
> >
> > So it will only read the single page we tried to access and won't
> > perturb the rest of the message encoded into subsequent pages in
> > file.
>
> Dave, you're being intentionally obtuse, aren't you?
>
> It's only that single page that *matters*. That's the page that the
> probe reveals the status of - but it's also the page that the probe
> then *changes* the status of.
It changes the state of it /after/ we've already got the information
we need from it. It's not up to date, it has to come from disk, we
return EAGAIN, which means it was not in the cache.
i.e. if we return EAGAIN, we've leaked the inforation the attacker
wants regardless of how the act of initiating readahead on the page
change the state of the page. Yes, it raises the complexity bar a
bit, and lowers the monitoring frequency somewhat, but that's about
it.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dominique Martinet @ 2019-01-11 7:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dave Chinner, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <CAHk-=wiqfAdmmE+pR3O5zs=xtkd6A6ShyyCwpwSZ+341L=zVYw@mail.gmail.com>
Linus Torvalds wrote on Thu, Jan 10, 2019:
> But those numbers aren't about the mincore() change. That's just from
> dropping caches.
>
> Now, what's the difference with the mincore change, and without? Is it
> actually measurable?
>
> Because that's all that matters: is the mincore change something you
> can even notice? Is it a big regression?
>
> The fact that things are slower when they are cold in the cache isn't
> the issue. The issue is whether the change to mincore semantics makes
> any difference to real loads.
mincore itself isn't used to reload the data, but is necessary to know
*what* you need to reload.
If you don't know what pages are hot, how can you preload them?
For small enough a database and with enough memory you can act stupid
and load the whole tables in cache, that's what I did because I was lazy
and only had a big mysql data set around. But the container warming up
automaton Dave mentioned and postgresql db preloading with pgfincore
explicitely depend on being able to tell what they need to preload.
pgfincore documentation states:
----
set of PostgreSQL functions to manage blocks in memory
Those functions let you know which and how many disk block from a
relation are in the page cache of the operating system, and eventually
write the result to a file. Then using this file, it is possible to
restore the page cache state for each block of the relation.
----
If you cannot dump an arbitrary "hot state" x, you cannot restore it.
This is all basically a repeat of the other subthread; sure precaching
itself doesn't need mincore and if you're all-knowing the syscall isn't
needed, but mere mortals need it.
If it's about the commit itself, vmtouch tells me 0 page in these
database files are in cache when I reboot to a 5.0-rc1 kernel and run
some queries, so the difference after a fresh boot is exactly what I
stated. I'm probably missing something but I'm not quite sure in what
situation the "new mincore" has any use right now.
--
Dominique Martinet | Asmadeus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dave Chinner @ 2019-01-11 7:20 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Dominique Martinet, Jiri Kosina, Matthew Wilcox,
Jann Horn, Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko,
Linux-MM, kernel list, Linux API
In-Reply-To: <6955E7C1-A61C-49F3-8BB6-0624D5A70BD6@amacapital.net>
On Thu, Jan 10, 2019 at 08:08:37PM -0800, Andy Lutomirski wrote:
> > On Jan 10, 2019, at 8:04 PM, Dave Chinner <david@fromorbit.com>
> > wrote:
> >
> >> On Thu, Jan 10, 2019 at 06:18:16PM -0800, Linus Torvalds
> >> wrote:
> >>> On Thu, Jan 10, 2019 at 6:03 PM Dave Chinner
> >>> <david@fromorbit.com> wrote:
> >>>
> >>>> On Thu, Jan 10, 2019 at 02:11:01PM -0800, Linus Torvalds
> >>>> wrote: And we *can* do sane things about RWF_NOWAIT. For
> >>>> example, we could start async IO on RWF_NOWAIT, and suddenly
> >>>> it would go from "probe the page cache" to "probe and fill",
> >>>> and be much harder to use as an attack vector..
> >>>
> >>> We can only do that if the application submits the read via
> >>> AIO and has an async IO completion reporting mechanism.
> >>
> >> Oh, no, you misunderstand.
> >>
> >> RWF_NOWAIT has a lot of situations where it will potentially
> >> return early (the DAX and direct IO ones have their own), but I
> >> was thinking of the one in generic_file_buffered_read(), which
> >> triggers when you don't find a page mapping. That looks like
> >> the obvious "probe page cache" case.
> >>
> >> But we could literally move that test down just a few lines.
> >> Let it start read-ahead.
> >>
> >> .. and then it will actually trigger on the *second* case
> >> instead, where we have
> >>
> >> if (!PageUptodate(page)) { if (iocb->ki_flags &
> >> IOCB_NOWAIT) { put_page(page); goto would_block;
> >> }
> >>
> >> and that's where RWF_MNOWAIT would act.
> >>
> >> It would still return EAGAIN.
> >>
> >> But it would have started filling the page cache. So now the
> >> act of probing would fill the page cache, and the attacker
> >> would be left high and dry - the fact that the page cache now
> >> exists is because of the attack, not because of whatever it was
> >> trying to measure.
> >>
> >> See?
> >
> > Except for fadvise(POSIX_FADV_RANDOM) which triggers this code
> > in page_cache_sync_readahead():
> >
> > /* be dumb */ if (filp && (filp->f_mode & FMODE_RANDOM))
> > { force_page_cache_readahead(mapping, filp, offset,
> > req_size); return; }
> >
> > So it will only read the single page we tried to access and
> > won't perturb the rest of the message encoded into subsequent
> > pages in file.
>
> There are two types of attacks. One is an intentional side
> channel where two cooperating processes communicate. This is,
> under some circumstances, a problem,
Yes, that's the covert communication channel that can cross container
and machine boundaries without any required privileges.
> but it’s not one
> we’re about to solve in general. The other is an attacker
> monitoring an unwilling process.
Which uses exactly the same mechanisms as the first case. i.e.
controlled invalidation and page cache residency monitoring.If we
aren't going to solve the first problem case, the we aren't going to
solve the second because they are one and the same problem...
However, I suspect you have misunderstood the monitoring mechanism
here - dispatch IO for this page doesn't prevent the information
leak about that page. It's when we return EAGAIN that we leak
information about page cache residency.
What Linus is attempting to do is perturb the nearby state of the
page cache by triggering async readahead in the EAGAIN case. Async
readahead will fill all the holes in readahead window and hence
destroy the information about where the page fault landed and
instantiated the page cache. That would prevent the attacker from
determining what code the executable is running as they would only
be able to check a single page in an executable at a time and that
makes the attack highly impractical.
But if the attacker uses FADV_RANDOM, readahead is only triggered
for the page the attacker is trying to read. Hence it does not
disturb the nearby page cache residency pattern the executable's
page faults left behind and so doesn't destroy the information that
they are trying to extract from the unwilling process.
Sure, Linus's change makes monitoring the executable file after
FADV_RANDOM a "read-once" mechanism. However, detection of what code
is executing is a repeated invalidate-and-sweep exercise to begin
with, so it basically doesn't change the information or the rate at
which the monitoring process can extract information from the file.
/me hasn't thought about this sort of stuff since he was running
page cache invalidation attacks on Irix system libraries way back in
2002 when he found a libc bug that killed the init process and
paniced the kernel.
This isn't my first rodeo - it's been well known for a long, long
time that the system page cache can be exploited to monitor
executing code...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-11 7:11 UTC (permalink / raw)
To: Dominique Martinet
Cc: Dave Chinner, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <20190111045750.GA27333@nautica>
On Thu, Jan 10, 2019 at 8:58 PM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> I get on average over a few queries approximately a real time of 350ms,
> 230ms and 220ms immediately after drop cache and service restart, and
> 150ms, 60ms and 60ms after a prefetch (hand-wavy average over 3 runs, I
> didn't have the patience to do proper testing).
> (In both cases, user/sys are less than 10ms; I don't see much difference
> there)
But those numbers aren't about the mincore() change. That's just from
dropping caches.
Now, what's the difference with the mincore change, and without? Is it
actually measurable?
Because that's all that matters: is the mincore change something you
can even notice? Is it a big regression?
The fact that things are slower when they are cold in the cache isn't
the issue. The issue is whether the change to mincore semantics makes
any difference to real loads.
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-11 7:08 UTC (permalink / raw)
To: Dave Chinner
Cc: Dominique Martinet, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <20190111040434.GN27534@dastard>
On Thu, Jan 10, 2019 at 8:04 PM Dave Chinner <david@fromorbit.com> wrote:
>
> So it will only read the single page we tried to access and won't
> perturb the rest of the message encoded into subsequent pages in
> file.
Dave, you're being intentionally obtuse, aren't you?
It's only that single page that *matters*. That's the page that the
probe reveals the status of - but it's also the page that the probe
then *changes* the status of.
See?
Think of it as "the act of measurement changes that which is
measured". And that makes the measurement pointless.
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dominique Martinet @ 2019-01-11 4:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dave Chinner, Jiri Kosina, Matthew Wilcox, Jann Horn,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <CAHk-=wip2CPrdOwgF0z4n2tsdW7uu+Egtcx9Mxxe3gPfPW_JmQ@mail.gmail.com>
Linus Torvalds wrote on Thu, Jan 10, 2019:
> On Thu, Jan 10, 2019 at 4:25 AM Dominique Martinet
> <asmadeus@codewreck.org> wrote:
> > Linus Torvalds wrote on Thu, Jan 10, 2019:
> > > (Except, of course, if somebody actually notices outside of tests.
> > > Which may well happen and just force us to revert that commit. But
> > > that's a separate issue entirely).
> >
> > Both Dave and I pointed at a couple of utilities that break with
> > this. nocache can arguably work with the new behaviour but will behave
> > differently; vmtouch on the other hand is no longer able to display
> > what's in cache or not - people use that for example to "warm up" a
> > container in page cache based on how it appears after it had been
> > running for a while is a pretty valid usecase to me.
>
> So honestly, the main reason I'm loath to revert is that yes, we know
> of theoretical differences, but they seem to all be
> performance-related.
I don't see what other use mincore could have, yes - even the
"debugging" use I gave is performance investigations and not hard
problems (and I probably would go straight to perf nowadays, you'd get
the info that the program doesn't use cache from the call graphs)
> It would be really good to hear numbers. Is the warm-up optimization
> something that changes things from 3ms to 3.5ms? Or does it change
> things from 3ms to half a second?
This is heavily workload and storage hardware dependant, so hard to give
some absolute value.
Trying with some big server, fast SSD, mysql and doing:
# echo 3 > /proc/sys/vm/drop_caches
# (optional) prefetch table and innodb files
# systemctl restart mariadb
# time mysql -q db "select * from mytable where id in $ENTRIES" > /dev/null
# time mysql -q db "select * from mytable where id in $ENTRIES2" > /dev/null
# time mysql -q db "select * from mytable where id in $ENTRIES3" > /dev/null
(where ENTRIES* are lists of 1000 id, and id is indexed; the table is 8GB
for 62590661 entries so 1000 entries is approx 128KB of data out of that
file)
I get on average over a few queries approximately a real time of 350ms,
230ms and 220ms immediately after drop cache and service restart, and
150ms, 60ms and 60ms after a prefetch (hand-wavy average over 3 runs, I
didn't have the patience to do proper testing).
(In both cases, user/sys are less than 10ms; I don't see much difference
there)
If I restart the service without dropping caches and redo the query I
get 60ms from the first query onwards so I must not be preloading
everything properly, some real script that would look all over a
container to properly restore the page cache would do better than me
blindly preloading a few files.
Either way, we're talking about a factor of 2-3 until the application has
been looking at most of the entries, and I didn't try to see how that
would look like on spinning disks or the kind of slow storage one would
get on VPS somewhere in the cloud - I'm sure someone with time to waste
could get much more impressive figures, but this already look pretty
worthwhile to me.
--
Dominique Martinet | Asmadeus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox