* Re: [PATCH 10/17] mm: rmap preparation for remap_anon_pages
From: Dr. David Alan Gilbert @ 2014-10-06 8:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrea Arcangeli, qemu-devel, KVM list, Linux Kernel Mailing List,
linux-mm, Linux API, Andres Lagar-Cavilla, Dave Hansen,
Paolo Bonzini, Rik van Riel, Mel Gorman, Andy Lutomirski,
Andrew Morton, Sasha Levin, Hugh Dickins, Peter Feiner,
\Dr. David Alan Gilbert\, Christopher Covington, Johannes Weiner,
Android Kernel Team, Robert
In-Reply-To: <CA+55aFx++R42L75ooE=Fmaem73=V=q7f6pYTcALxgrA1y98G-A@mail.gmail.com>
* Linus Torvalds (torvalds@linux-foundation.org) wrote:
> On Fri, Oct 3, 2014 at 10:08 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> >
> > Overall this looks a fairly small change to the rmap code, notably
> > less intrusive than the nonlinear vmas created by remap_file_pages.
>
> Considering that remap_file_pages() was an unmitigated disaster, and
> -mm has a patch to remove it entirely, I'm not at all convinced this
> is a good argument.
>
> We thought remap_file_pages() was a good idea, and it really really
> really wasn't. Almost nobody used it, why would the anonymous page
> case be any different?
I've posted code that uses this interface to qemu-devel and it works nicely;
so chalk up at least one user.
For the postcopy case I'm using it for, we need to place a page, atomically
some thread might try and access it, and must either
1) get caught by userfault etc or
2) must succeed in it's access
and we'll have that happening somewhere between thousands and millions of times
to pages in no particular order, so we need to avoid creating millions of mappings.
Dave
>
> Linus
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH RFC] introduce ioctl to completely invalidate page cache
From: Jan Kara @ 2014-10-06 8:06 UTC (permalink / raw)
To: Jens Axboe
Cc: Thanos Makatos, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
bfields-uC3wQj2KruNg9hUCZPvPmw, jack-AlSwsSmVLrQ
In-Reply-To: <542DAEAC.8010203-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
On Thu 02-10-14 13:59:40, Jens Axboe wrote:
> On 10/02/2014 10:09 AM, Thanos Makatos wrote:
> > This patch introduces a new ioctl called BLKFLUSHBUFS2, which is pretty
> > similar to BLKFLUSHBUFS except that is also invalidates the page cache.
> > This allows for a complete invalidation of the cached data of a
> > particular block device, which might be useful for cases like
> > synchronising the caches of an iSCSI block device used by multiple
> > hosts.
> >
> > Signed-off-by: Thanos Makatos <thanos.makatos-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> > ---
> > block/compat_ioctl.c | 1 +
> > block/ioctl.c | 13 +++++++++++--
> > include/uapi/linux/fs.h | 1 +
> > 3 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
> > index 18b282c..672388ab 100644
> > --- a/block/compat_ioctl.c
> > +++ b/block/compat_ioctl.c
> > @@ -688,6 +688,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> > case BLKDISCARDZEROES:
> > return compat_put_uint(arg, bdev_discard_zeroes_data(bdev));
> > case BLKFLSBUF:
> > + case BLKFLSBUF2:
> > case BLKROSET:
> > case BLKDISCARD:
> > case BLKSECDISCARD:
> > diff --git a/block/ioctl.c b/block/ioctl.c
> > index d6cda81..0c427a7 100644
> > --- a/block/ioctl.c
> > +++ b/block/ioctl.c
> > @@ -268,6 +268,12 @@ static inline int is_unrecognized_ioctl(int ret)
> > ret == -ENOIOCTLCMD;
> > }
> >
> > +static void flush_buffer_cache(struct block_device *bdev)
> > +{
> > + fsync_bdev(bdev);
> > + invalidate_bdev(bdev);
> > +}
> > +
> > /*
> > * always keep this in sync with compat_blkdev_ioctl()
> > */
> > @@ -282,6 +288,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
> >
> > switch(cmd) {
> > case BLKFLSBUF:
> > + case BLKFLSBUF2:
> > if (!capable(CAP_SYS_ADMIN))
> > return -EACCES;
> >
> > @@ -289,8 +296,10 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
> > if (!is_unrecognized_ioctl(ret))
> > return ret;
> >
> > - fsync_bdev(bdev);
> > - invalidate_bdev(bdev);
> > + flush_buffer_cache(bdev);
> > + if (BLKFLSBUF2 == cmd)
> > + return invalidate_inode_pages2(
> > + bdev->bd_inode->i_mapping);
> > return 0;
>
> We're currently ignoring the buffer cache sync and invalidation (which
> is odd), but at least being consistent would be good.
Well, invalidate_bdev() doesn't return anything. And
invalidate_mapping_pages() inside invalidate_bdev() returns only number of
invalidated pages. I don't think there's any value in returning that.
OTOH invalidate_inode_pages2() returns 0 / -EBUSY / other error when
invalidation of some page fails so returning that seems useful.
> Might also need a filemap_write_and_wait() to sync before invalidation.
That's what fsync_bdev() is doing under the hoods. Sometimes I'm not
sure whether all these wrappers are useful...
Trond also had a comment that if we extended the ioctl to work for all
inodes (not just blkdev) and allowed some additional flags of what needs to
be invalidated, the new ioctl would be also useful to NFS userspace - see
Trond's email at
http://www.spinics.net/lists/linux-fsdevel/msg78917.html
and the following thread. I would prefer to cover that usecase when we are
introducing new invalidation ioctl. Have you considered that Thanos?
Honza
--
Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 12/17] mm: sys_remap_anon_pages
From: Andi Kleen @ 2014-10-04 13:13 UTC (permalink / raw)
To: Andrea Arcangeli; +Cc: linux-kernel, linux-mm, linux-api, Linus Torvalds
In-Reply-To: <1412356087-16115-13-git-send-email-aarcange@redhat.com>
Andrea Arcangeli <aarcange@redhat.com> writes:
> This new syscall will move anon pages across vmas, atomically and
> without touching the vmas.
>
> It only works on non shared anonymous pages because those can be
> relocated without generating non linear anon_vmas in the rmap code.
...
> It is an alternative to mremap.
Why a new syscall? Couldn't mremap do this transparently?
-Andi
--
ak@linux.intel.com -- Speaking for myself only
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-04 8:13 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel@vger.kernel.org, Ingo Molnar, Kees Cook,
Andrea Arcangeli, Erik Bosman, H. Peter Anvin, Linux API,
Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <CALCETrVtK6w4smnRCTED=csAyt3WNNOaZE_WRzvECuSx260X3w@mail.gmail.com>
On Fri, Oct 03, 2014 at 02:15:24PM -0700, Andy Lutomirski wrote:
> On Fri, Oct 3, 2014 at 2:12 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, Oct 03, 2014 at 02:04:53PM -0700, Andy Lutomirski wrote:
> >> On Fri, Oct 3, 2014 at 2:02 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> >> > Something like so.. slightly less ugly and possibly with more
> >> > complicated conditions setting the cr4 if you want to fix tsc vs seccomp
> >> > as well.
> >>
> >> This will crash anything that tries rdpmc in an allow-everything
> >> seccomp sandbox. It's also not very compatible with my grand scheme
> >> of allowing rdtsc to be turned off without breaking clock_gettime. :)
> >
> > Well, we clear cap_user_rdpmc, so everybody who still tries it gets what
> > he deserves, no problem there.
>
> Oh, interesting.
>
> To continue playing devil's advocate, what if you do perf_event_open,
> then mmap it, then start the seccomp sandbox?
We update that cap bit on every update to the self-monitor state, and in
a perfect world people would also check the cap bit every time they try
and read it, and fall back to the syscall. So we could just clear it..
but I can imagine reality ruining things here.
> My draft patches are currently tracking the number of perf_event mmaps
> per mm. I'm not thrilled with it, but it's straightforward. And I
> still need to benchmark cr4 writes, which is tedious, because I can't
> do it from user code.
Should be fairly straight fwd from kernel space, get a tsc stamp,
read+write cr4 1000 times, get another tsc read, and maybe do that
several times. No?
^ permalink raw reply
* Re: [PATCH 08/17] mm: madvise MADV_USERFAULT
From: Mike Hommey @ 2014-10-03 23:13 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: qemu-devel, kvm, linux-kernel, linux-mm, linux-api,
Linus Torvalds, Andres Lagar-Cavilla, Dave Hansen, Paolo Bonzini,
Rik van Riel, Mel Gorman, Andy Lutomirski, Andrew Morton,
Sasha Levin, Hugh Dickins, Peter Feiner,
\"Dr. David Alan Gilbert\", Christopher Covington,
Johannes Weiner, Android Kernel Team, Robert Love,
Dmitry Adamushko, Neil Brown, Taras Glek, Jan Kara
In-Reply-To: <1412356087-16115-9-git-send-email-aarcange@redhat.com>
On Fri, Oct 03, 2014 at 07:07:58PM +0200, Andrea Arcangeli wrote:
> MADV_USERFAULT is a new madvise flag that will set VM_USERFAULT in the
> vma flags. Whenever VM_USERFAULT is set in an anonymous vma, if
> userland touches a still unmapped virtual address, a sigbus signal is
> sent instead of allocating a new page. The sigbus signal handler will
> then resolve the page fault in userland by calling the
> remap_anon_pages syscall.
What does "unmapped virtual address" mean in this context?
Mike
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 21:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003211204.GQ10583-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Fri, Oct 3, 2014 at 2:12 PM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Fri, Oct 03, 2014 at 02:04:53PM -0700, Andy Lutomirski wrote:
>> On Fri, Oct 3, 2014 at 2:02 PM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
>> > Something like so.. slightly less ugly and possibly with more
>> > complicated conditions setting the cr4 if you want to fix tsc vs seccomp
>> > as well.
>>
>> This will crash anything that tries rdpmc in an allow-everything
>> seccomp sandbox. It's also not very compatible with my grand scheme
>> of allowing rdtsc to be turned off without breaking clock_gettime. :)
>
> Well, we clear cap_user_rdpmc, so everybody who still tries it gets what
> he deserves, no problem there.
Oh, interesting.
To continue playing devil's advocate, what if you do perf_event_open,
then mmap it, then start the seccomp sandbox?
My draft patches are currently tracking the number of perf_event mmaps
per mm. I'm not thrilled with it, but it's straightforward. And I
still need to benchmark cr4 writes, which is tedious, because I can't
do it from user code.
--Andy
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 21:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <CALCETrW7OCuAiK31iRvXgXJfcf3FE4GKjpKQ0doWFyUpETzT9A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Oct 03, 2014 at 02:04:53PM -0700, Andy Lutomirski wrote:
> On Fri, Oct 3, 2014 at 2:02 PM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > Something like so.. slightly less ugly and possibly with more
> > complicated conditions setting the cr4 if you want to fix tsc vs seccomp
> > as well.
>
> This will crash anything that tries rdpmc in an allow-everything
> seccomp sandbox. It's also not very compatible with my grand scheme
> of allowing rdtsc to be turned off without breaking clock_gettime. :)
Well, we clear cap_user_rdpmc, so everybody who still tries it gets what
he deserves, no problem there.
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 21:04 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003210213.GG6324-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Fri, Oct 3, 2014 at 2:02 PM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Fri, Oct 03, 2014 at 10:44:43PM +0200, Peter Zijlstra wrote:
>> On Fri, Oct 03, 2014 at 01:27:52PM -0700, Andy Lutomirski wrote:
>> > On Fri, Oct 3, 2014 at 1:22 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> > >
>> > > We could make the rule be that RDPMC is enabled if a perf event is
>> > > mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
>> > > there's an actual performance issue first. Ideally we can get this
>> > > all working with no API or ABI change at all.
>> >
>> > No, we can't use that rule. But we could say that RDPMC is enabled if
>> > a perf event is mmapped and no thread in the mm uses seccomp. I'll
>> > grumble a little bit about adding yet another piece of seccomp state.
>>
>> Well, we could simply disable the RDPMC for everything TIF_SECCOMP.
>> Should be fairly straight fwd.
>
>
> Something like so.. slightly less ugly and possibly with more
> complicated conditions setting the cr4 if you want to fix tsc vs seccomp
> as well.
This will crash anything that tries rdpmc in an allow-everything
seccomp sandbox. It's also not very compatible with my grand scheme
of allowing rdtsc to be turned off without breaking clock_gettime. :)
I'll send out a real set of patches in the next few days. I'll even
try to benchmark them :)
>
> ---
> arch/x86/kernel/cpu/perf_event.c | 13 ++++++++++++-
> arch/x86/kernel/process.c | 24 +++++++++++++++++-------
> 2 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index 16c73022306e..cfc42ff5d901 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1869,6 +1869,17 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
> return count;
> }
>
> +void perf_change_rdpmc(bool on, unsigned long *cr4)
> +{
> + if (x86_pmu.attr_rdpmc_broken)
> + return;
> +
> + if (on)
> + *cr4 |= X86_CR4_PCE;
> + else
> + *cr4 &= ~X86_CR4_PCE;
> +}
> +
> static DEVICE_ATTR(rdpmc, S_IRUSR | S_IWUSR, get_attr_rdpmc, set_attr_rdpmc);
>
> static struct attribute *x86_pmu_attrs[] = {
> @@ -1928,7 +1939,7 @@ void arch_perf_update_userpage(struct perf_event_mmap_page *userpg, u64 now)
>
> userpg->cap_user_time = 0;
> userpg->cap_user_time_zero = 0;
> - userpg->cap_user_rdpmc = x86_pmu.attr_rdpmc;
> + userpg->cap_user_rdpmc = x86_pmu.attr_rdpmc && test_thread_flag(TIF_SECCOMP);
> userpg->pmc_width = x86_pmu.cntval_bits;
>
> if (!sched_clock_stable())
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index e127ddaa2d5a..b74c0400851e 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -201,12 +201,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
> struct tss_struct *tss)
> {
> struct thread_struct *prev, *next;
> + struct thread_info *pi, *ni;
>
> prev = &prev_p->thread;
> next = &next_p->thread;
>
> - if (test_tsk_thread_flag(prev_p, TIF_BLOCKSTEP) ^
> - test_tsk_thread_flag(next_p, TIF_BLOCKSTEP)) {
> + pi = task_thread_info(prev_p);
> + ni = task_thread_info(next_p);
> +
> + if ((pi->flags & _TIF_BLOCKSTEP) ^ (ni->flags & _TIF_BLOCKSTEP)) {
> unsigned long debugctl = get_debugctlmsr();
>
> debugctl &= ~DEBUGCTLMSR_BTF;
> @@ -216,13 +219,20 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
> update_debugctlmsr(debugctl);
> }
>
> - if (test_tsk_thread_flag(prev_p, TIF_NOTSC) ^
> - test_tsk_thread_flag(next_p, TIF_NOTSC)) {
> + if ((pi->flags & (_TIF_NOTSC | _TIF_SECCOMP)) ^
> + (ni->flags & (_TIF_NOTSC | _TIF_SECCOMP))) {
> + extern void perf_change_rdpmc(bool, unsigned long *);
> + unsigned long cr4 = read_cr4();
> +
> /* prev and next are different */
> - if (test_tsk_thread_flag(next_p, TIF_NOTSC))
> - hard_disable_TSC();
> + if (ni->flags & _TIF_NOTSC)
> + cr4 |= X86_CR4_TSD;
> else
> - hard_enable_TSC();
> + cr4 &= ~X86_CR4_TSD;
> +
> + perf_change_rdpmc(!(ni->flags & _TIF_SECCOMP), &cr4);
> +
> + write_cr4(cr4);
> }
>
> if (test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 21:04 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003210213.GG6324-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Fri, Oct 03, 2014 at 11:02:13PM +0200, Peter Zijlstra wrote:
> @@ -1928,7 +1939,7 @@ void arch_perf_update_userpage(struct perf_event_mmap_page *userpg, u64 now)
>
> userpg->cap_user_time = 0;
> userpg->cap_user_time_zero = 0;
> - userpg->cap_user_rdpmc = x86_pmu.attr_rdpmc;
> + userpg->cap_user_rdpmc = x86_pmu.attr_rdpmc && test_thread_flag(TIF_SECCOMP);
&& !test_thread_flag(TIF_SECCOMP) would probably work better
> userpg->pmc_width = x86_pmu.cntval_bits;
>
> if (!sched_clock_stable())
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 21:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003204443.GP10583-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Fri, Oct 03, 2014 at 10:44:43PM +0200, Peter Zijlstra wrote:
> On Fri, Oct 03, 2014 at 01:27:52PM -0700, Andy Lutomirski wrote:
> > On Fri, Oct 3, 2014 at 1:22 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> > >
> > > We could make the rule be that RDPMC is enabled if a perf event is
> > > mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
> > > there's an actual performance issue first. Ideally we can get this
> > > all working with no API or ABI change at all.
> >
> > No, we can't use that rule. But we could say that RDPMC is enabled if
> > a perf event is mmapped and no thread in the mm uses seccomp. I'll
> > grumble a little bit about adding yet another piece of seccomp state.
>
> Well, we could simply disable the RDPMC for everything TIF_SECCOMP.
> Should be fairly straight fwd.
Something like so.. slightly less ugly and possibly with more
complicated conditions setting the cr4 if you want to fix tsc vs seccomp
as well.
---
arch/x86/kernel/cpu/perf_event.c | 13 ++++++++++++-
arch/x86/kernel/process.c | 24 +++++++++++++++++-------
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 16c73022306e..cfc42ff5d901 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1869,6 +1869,17 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
return count;
}
+void perf_change_rdpmc(bool on, unsigned long *cr4)
+{
+ if (x86_pmu.attr_rdpmc_broken)
+ return;
+
+ if (on)
+ *cr4 |= X86_CR4_PCE;
+ else
+ *cr4 &= ~X86_CR4_PCE;
+}
+
static DEVICE_ATTR(rdpmc, S_IRUSR | S_IWUSR, get_attr_rdpmc, set_attr_rdpmc);
static struct attribute *x86_pmu_attrs[] = {
@@ -1928,7 +1939,7 @@ void arch_perf_update_userpage(struct perf_event_mmap_page *userpg, u64 now)
userpg->cap_user_time = 0;
userpg->cap_user_time_zero = 0;
- userpg->cap_user_rdpmc = x86_pmu.attr_rdpmc;
+ userpg->cap_user_rdpmc = x86_pmu.attr_rdpmc && test_thread_flag(TIF_SECCOMP);
userpg->pmc_width = x86_pmu.cntval_bits;
if (!sched_clock_stable())
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index e127ddaa2d5a..b74c0400851e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -201,12 +201,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
struct tss_struct *tss)
{
struct thread_struct *prev, *next;
+ struct thread_info *pi, *ni;
prev = &prev_p->thread;
next = &next_p->thread;
- if (test_tsk_thread_flag(prev_p, TIF_BLOCKSTEP) ^
- test_tsk_thread_flag(next_p, TIF_BLOCKSTEP)) {
+ pi = task_thread_info(prev_p);
+ ni = task_thread_info(next_p);
+
+ if ((pi->flags & _TIF_BLOCKSTEP) ^ (ni->flags & _TIF_BLOCKSTEP)) {
unsigned long debugctl = get_debugctlmsr();
debugctl &= ~DEBUGCTLMSR_BTF;
@@ -216,13 +219,20 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
update_debugctlmsr(debugctl);
}
- if (test_tsk_thread_flag(prev_p, TIF_NOTSC) ^
- test_tsk_thread_flag(next_p, TIF_NOTSC)) {
+ if ((pi->flags & (_TIF_NOTSC | _TIF_SECCOMP)) ^
+ (ni->flags & (_TIF_NOTSC | _TIF_SECCOMP))) {
+ extern void perf_change_rdpmc(bool, unsigned long *);
+ unsigned long cr4 = read_cr4();
+
/* prev and next are different */
- if (test_tsk_thread_flag(next_p, TIF_NOTSC))
- hard_disable_TSC();
+ if (ni->flags & _TIF_NOTSC)
+ cr4 |= X86_CR4_TSD;
else
- hard_enable_TSC();
+ cr4 &= ~X86_CR4_TSD;
+
+ perf_change_rdpmc(!(ni->flags & _TIF_SECCOMP), &cr4);
+
+ write_cr4(cr4);
}
if (test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
^ permalink raw reply related
* Re: [PATCH 01/17] mm: gup: add FOLL_TRIED
From: Paolo Bonzini @ 2014-10-03 20:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrea Arcangeli, qemu-devel, KVM list, Linux Kernel Mailing List,
linux-mm, Linux API, Andres Lagar-Cavilla, Dave Hansen,
Rik van Riel, Mel Gorman, Andy Lutomirski, Andrew Morton,
Sasha Levin, Hugh Dickins, Peter Feiner, \Dr. David Alan Gilbert,
Christopher Covington, Johannes Weiner, Android Kernel Team,
Robert Love
In-Reply-To: <CA+55aFwEO9=ieSb0uipfQ+qP9nP1ps=NCTe0HL9arK1cRYaJPg@mail.gmail.com>
> This needs more explanation than that one-liner comment. Make the
> commit message explain why the new FOLL_TRIED flag exists.
This patch actually is extracted from a 3.18 commit in the KVM tree,
https://git.kernel.org/cgit/virt/kvm/kvm.git/commit/?h=next&id=234b239b.
Here is how that patch uses the flag:
/*
* The previous call has now waited on the IO. Now we can
* retry and complete. Pass TRIED to ensure we do not re
* schedule async IO (see e.g. filemap_fault).
*/
down_read(&mm->mmap_sem);
npages = __get_user_pages(tsk, mm, addr, 1, flags | FOLL_TRIED,
pagep, NULL, NULL);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 20:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel@vger.kernel.org, Ingo Molnar, Kees Cook,
Andrea Arcangeli, Erik Bosman, H. Peter Anvin, Linux API,
Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003204256.GO10583@worktop.programming.kicks-ass.net>
On Fri, Oct 3, 2014 at 1:42 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Oct 03, 2014 at 01:22:22PM -0700, Andy Lutomirski wrote:
>> > So the problem with the default deny is that its:
>> > 1) pointless -- the attacker can do sys_perf_event_open() just fine;
>>
>> Not if the attacker is in a seccomp sandbox.
>
> Clearly :-)
>
>> > 2) and expensive -- the people trying to measure performance get the
>> > penalty of the CR4 write.
>>
>> Does this matter for performance measuring? I'm not 100% clear on how all
>> the perf_event stuff gets used in practice, but, by my very vague
>> understanding, there are two main workflows:
>>
>> a) perf record, etc: one process creates a ringbuffer and wakes up
>> rarely to record the contents. The process being recorded doesn't
>> have a perf_event mapped, so the cr4 switch will only happen when
>> waking up the perf process.
>>
>> perf record prints stuff like "[ perf record: Woken up 1 times to
>> write data ]", which seems to confirm my understanding.
>>
>> b) self-monitoring. A task mmaps a perf_event, does rdpmc, does
>> something, and does rdpmc again. In that case, there's no context
>> switch.
>
> Agreed, but with the default disable, the self-monitoring task will get
> CR4 writes to its context switches and will be slower.
>
> Whereas if we default on and only disable with seccomp then only the
> seccomp tasks will suffer the burden of a CR4 write at context switch
> time.
>
> And I don't see a security implication in the default.
I don't see a security implication, but we can fight over whether
seccomp performance or perf self monitoring performance is more
important. I spent a bunch of time making seccomp *much* faster for
3.18, and things like Chromium (the browser) could have lots of
context switched into and out of seccomp. :)
I suspect that the overhead only really matters when running as a VM guest.
Hmm. Can we switch lazily? I don't really like the idea, but
non-seccomp, non-perf-event tasks could, in principle, just inherit
the PCE value from whatever had the CPU last.
>
>> > So I would suggest a default on, but allow a disable for the seccomp
>> > users, which might have also disabled the syscall. Note that is is
>> > possible to disable RDPMC while still allowing the syscall.
>>
>> Disabling RDPMC per-process while still allowing the syscall will need
>> a bunch of work, right?
>
> Not much, all you need to do is make
> perf_event_mmmap_page::cap_user_rdpmc be 0 and userspace should never
> use rdpmc. Currently we set that bit based on the global sysfs rdpmc
> value, but we could easy bitwise AND it with a per process value.
>
>> What happens if the same perf_event is mapped
>> by two different users?
>
> Not entirely clear on what you mean here.
Can you open a perf_event fd, fork, and mmap it in the child? What if
you map it in the parent and the child? Then cap_user_rdpmc has to
match for both mappings. Or is this impossible?
>
>> We could make the rule be that RDPMC is enabled if a perf event is
>> mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
>> there's an actual performance issue first. Ideally we can get this
>> all working with no API or ABI change at all.
>
> Well I just want to not have extra CR4 writes by default (and by default
> I don't care about seccomp).
>
>> P.S. Hey, Intel, let us context switch RDPMC accessibility of the
>> individual counters, please :)
>
> Ha sure, make it more complicated why don't you ;-) But yes, that has
> come up before.
--Andy
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 20:46 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003204443.GP10583-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Fri, Oct 3, 2014 at 1:44 PM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Fri, Oct 03, 2014 at 01:27:52PM -0700, Andy Lutomirski wrote:
>> On Fri, Oct 3, 2014 at 1:22 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> >
>> > We could make the rule be that RDPMC is enabled if a perf event is
>> > mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
>> > there's an actual performance issue first. Ideally we can get this
>> > all working with no API or ABI change at all.
>>
>> No, we can't use that rule. But we could say that RDPMC is enabled if
>> a perf event is mmapped and no thread in the mm uses seccomp. I'll
>> grumble a little bit about adding yet another piece of seccomp state.
>
> Well, we could simply disable the RDPMC for everything TIF_SECCOMP.
> Should be fairly straight fwd.
That won't work. I bet there are plenty of existing users of fairly
wide-open seccomp sandboxes that allow perf_event in.
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 20:44 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <CALCETrWfrWpdMCAYySMAMGCHU3XRkNGmeMTECTE=PXQUfjGPZA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Oct 03, 2014 at 01:27:52PM -0700, Andy Lutomirski wrote:
> On Fri, Oct 3, 2014 at 1:22 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> >
> > We could make the rule be that RDPMC is enabled if a perf event is
> > mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
> > there's an actual performance issue first. Ideally we can get this
> > all working with no API or ABI change at all.
>
> No, we can't use that rule. But we could say that RDPMC is enabled if
> a perf event is mmapped and no thread in the mm uses seccomp. I'll
> grumble a little bit about adding yet another piece of seccomp state.
Well, we could simply disable the RDPMC for everything TIF_SECCOMP.
Should be fairly straight fwd.
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 20:42 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <CALCETrVvFP66s5XOmSKaC8Vq73=uh11819HOOLkVTu7jJZotew-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Oct 03, 2014 at 01:22:22PM -0700, Andy Lutomirski wrote:
> > So the problem with the default deny is that its:
> > 1) pointless -- the attacker can do sys_perf_event_open() just fine;
>
> Not if the attacker is in a seccomp sandbox.
Clearly :-)
> > 2) and expensive -- the people trying to measure performance get the
> > penalty of the CR4 write.
>
> Does this matter for performance measuring? I'm not 100% clear on how all
> the perf_event stuff gets used in practice, but, by my very vague
> understanding, there are two main workflows:
>
> a) perf record, etc: one process creates a ringbuffer and wakes up
> rarely to record the contents. The process being recorded doesn't
> have a perf_event mapped, so the cr4 switch will only happen when
> waking up the perf process.
>
> perf record prints stuff like "[ perf record: Woken up 1 times to
> write data ]", which seems to confirm my understanding.
>
> b) self-monitoring. A task mmaps a perf_event, does rdpmc, does
> something, and does rdpmc again. In that case, there's no context
> switch.
Agreed, but with the default disable, the self-monitoring task will get
CR4 writes to its context switches and will be slower.
Whereas if we default on and only disable with seccomp then only the
seccomp tasks will suffer the burden of a CR4 write at context switch
time.
And I don't see a security implication in the default.
> > So I would suggest a default on, but allow a disable for the seccomp
> > users, which might have also disabled the syscall. Note that is is
> > possible to disable RDPMC while still allowing the syscall.
>
> Disabling RDPMC per-process while still allowing the syscall will need
> a bunch of work, right?
Not much, all you need to do is make
perf_event_mmmap_page::cap_user_rdpmc be 0 and userspace should never
use rdpmc. Currently we set that bit based on the global sysfs rdpmc
value, but we could easy bitwise AND it with a per process value.
> What happens if the same perf_event is mapped
> by two different users?
Not entirely clear on what you mean here.
> We could make the rule be that RDPMC is enabled if a perf event is
> mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
> there's an actual performance issue first. Ideally we can get this
> all working with no API or ABI change at all.
Well I just want to not have extra CR4 writes by default (and by default
I don't care about seccomp).
> P.S. Hey, Intel, let us context switch RDPMC accessibility of the
> individual counters, please :)
Ha sure, make it more complicated why don't you ;-) But yes, that has
come up before.
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 20:27 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel@vger.kernel.org, Ingo Molnar, Kees Cook,
Andrea Arcangeli, Erik Bosman, H. Peter Anvin, Linux API,
Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <CALCETrVvFP66s5XOmSKaC8Vq73=uh11819HOOLkVTu7jJZotew@mail.gmail.com>
On Fri, Oct 3, 2014 at 1:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> We could make the rule be that RDPMC is enabled if a perf event is
> mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
> there's an actual performance issue first. Ideally we can get this
> all working with no API or ABI change at all.
No, we can't use that rule. But we could say that RDPMC is enabled if
a perf event is mmapped and no thread in the mm uses seccomp. I'll
grumble a little bit about adding yet another piece of seccomp state.
--Andy
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 20:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <20141003201409.GM10583-IIpfhp3q70z/8w/KjCw3T+5/BudmfyzbbVWyRVo5IupeoWH0uzbU5w@public.gmane.org>
On Fri, Oct 3, 2014 at 1:14 PM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Fri, Oct 03, 2014 at 10:27:47AM -0700, Andy Lutomirski wrote:
>> [adding linux-api. whoops.]
>>
>> On Fri, Oct 3, 2014 at 10:18 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> > PR_SET_TSC / PR_TSC_SIGSEGV is a security feature to prevent heavily
>> > sandboxed programs from learning the time, presumably to avoid
>> > disclosing the wall clock and to make timing attacks much harder to
>> > exploit.
>> >
>> > Unfortunately, this feature is very insecure, for multiple reasons,
>> > and has probably been insecure since before it was written.
>> >
>> > Weakness 1: Before Linux 3.16, the vvar page and the HPET (!) were
>> > part of the kernel's fixmap, so any user process could read them.
>> > The vvar page contains low-resolution timing information (with real
>> > wall clock and frequency data), and the HPET can be used for high
>> > precision timing. Even in Linux 3.16, there clean way to disable
>> > access to these pages.
>> >
>> > Weakness 2: On most configurations, most or all userspace processes
>> > have unrestricted access to RDPMC, which is even better than RDTSC
>> > for exploiting timing attacks.
>> >
>> > I would like to fix both of these issues. I want to deny access to
>> > RDPMC to processes that haven't asked for access via
>> > perf_event_open. I also want to implement real TSC blocking, which
>> > will require some vdso enhancements
>
> So the problem with the default deny is that its:
> 1) pointless -- the attacker can do sys_perf_event_open() just fine;
Not if the attacker is in a seccomp sandbox.
> 2) and expensive -- the people trying to measure performance get the
> penalty of the CR4 write.
Does this matter for performance measuring? I'm not 100% clear on how all
the perf_event stuff gets used in practice, but, by my very vague
understanding, there are two main workflows:
a) perf record, etc: one process creates a ringbuffer and wakes up
rarely to record the contents. The process being recorded doesn't
have a perf_event mapped, so the cr4 switch will only happen when
waking up the perf process.
perf record prints stuff like "[ perf record: Woken up 1 times to
write data ]", which seems to confirm my understanding.
b) self-monitoring. A task mmaps a perf_event, does rdpmc, does
something, and does rdpmc again. In that case, there's no context
switch.
>
> So I would suggest a default on, but allow a disable for the seccomp
> users, which might have also disabled the syscall. Note that is is
> possible to disable RDPMC while still allowing the syscall.
Disabling RDPMC per-process while still allowing the syscall will need
a bunch of work, right? What happens if the same perf_event is mapped
by two different users?
We could make the rule be that RDPMC is enabled if a perf event is
mmapped or TIF_SECCOMP is clear, but I'd prefer to be convinced that
there's an actual performance issue first. Ideally we can get this
all working with no API or ABI change at all.
P.S. Hey, Intel, let us context switch RDPMC accessibility of the
individual counters, please :)
--Andy
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 20:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrea Arcangeli, Paul Mackerras, Arnaldo Carvalho de Melo,
X86 ML, linux-kernel@vger.kernel.org, Ingo Molnar, Kees Cook,
Erik Bosman, H. Peter Anvin, Linux API, Michael Kerrisk-manpages
In-Reply-To: <CALCETrWP7QSb4dFh_phUK+7P-XookgL8UXgvHi-e4icukUF2Og@mail.gmail.com>
On Fri, Oct 03, 2014 at 10:59:15AM -0700, Andy Lutomirski wrote:
> RDPMC is controlled by CR4.PCE, whereas RDTSC is controlled by
> CR4.TSD,
This is my understanding too, they're separate things.
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Peter Zijlstra @ 2014-10-03 20:14 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Kees Cook, Andrea Arcangeli, Erik Bosman, H. Peter Anvin,
Linux API, Michael Kerrisk-manpages, Paul Mackerras,
Arnaldo Carvalho de Melo, X86 ML
In-Reply-To: <CALCETrUfCrvidOS6VvUpWFAcHUrPUs58zSQqGRC5UOTS=E37rw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Oct 03, 2014 at 10:27:47AM -0700, Andy Lutomirski wrote:
> [adding linux-api. whoops.]
>
> On Fri, Oct 3, 2014 at 10:18 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> > PR_SET_TSC / PR_TSC_SIGSEGV is a security feature to prevent heavily
> > sandboxed programs from learning the time, presumably to avoid
> > disclosing the wall clock and to make timing attacks much harder to
> > exploit.
> >
> > Unfortunately, this feature is very insecure, for multiple reasons,
> > and has probably been insecure since before it was written.
> >
> > Weakness 1: Before Linux 3.16, the vvar page and the HPET (!) were
> > part of the kernel's fixmap, so any user process could read them.
> > The vvar page contains low-resolution timing information (with real
> > wall clock and frequency data), and the HPET can be used for high
> > precision timing. Even in Linux 3.16, there clean way to disable
> > access to these pages.
> >
> > Weakness 2: On most configurations, most or all userspace processes
> > have unrestricted access to RDPMC, which is even better than RDTSC
> > for exploiting timing attacks.
> >
> > I would like to fix both of these issues. I want to deny access to
> > RDPMC to processes that haven't asked for access via
> > perf_event_open. I also want to implement real TSC blocking, which
> > will require some vdso enhancements
So the problem with the default deny is that its:
1) pointless -- the attacker can do sys_perf_event_open() just fine;
2) and expensive -- the people trying to measure performance get the
penalty of the CR4 write.
So I would suggest a default on, but allow a disable for the seccomp
users, which might have also disabled the syscall. Note that is is
possible to disable RDPMC while still allowing the syscall.
^ permalink raw reply
* Re: [PATCH v3 3/5] selftests/ipc: change test to use ksft framework
From: Davidlohr Bueso @ 2014-10-03 20:01 UTC (permalink / raw)
To: Shuah Khan
Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
colin.king-Z7WLFzj8eWMS+FvcfC7Uqw, luto-kltTT9wpgjJwATOyAt5JVQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <542EFC36.1010306-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Fri, 2014-10-03 at 13:42 -0600, Shuah Khan wrote:
> On 10/03/2014 11:39 AM, Davidlohr Bueso wrote:
> > On Fri, 2014-10-03 at 09:36 -0600, Shuah Khan wrote:
> >> msgque.key = ftok(argv[0], 822155650);
> >> if (msgque.key == -1) {
> >> - printf("Can't make key\n");
> >> - return -errno;
> >> + printf("Can't make key: %d\n", -errno);
> >
> > So printing a numeric value is quite useless when users actually run
> > into these errors -- which is why I like err() so much. How about using
> > strerror() instead?
> >
>
> Yes. using perror() does give better information. There are other
> places in this file that use errno. How about I make that a separate
> patch and catch all of them at once to use perror() as a follow-up
> change? That way I fix all at once without adding more changes to
> this patch.
Sounds good.
^ permalink raw reply
* Re: [PATCH v3 3/5] selftests/ipc: change test to use ksft framework
From: Shuah Khan @ 2014-10-03 19:42 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
colin.king-Z7WLFzj8eWMS+FvcfC7Uqw, luto-kltTT9wpgjJwATOyAt5JVQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Shuah Khan
In-Reply-To: <1412357993.20838.4.camel-dxKd5G12XOI1EaDjlw0dpg@public.gmane.org>
On 10/03/2014 11:39 AM, Davidlohr Bueso wrote:
> On Fri, 2014-10-03 at 09:36 -0600, Shuah Khan wrote:
>> msgque.key = ftok(argv[0], 822155650);
>> if (msgque.key == -1) {
>> - printf("Can't make key\n");
>> - return -errno;
>> + printf("Can't make key: %d\n", -errno);
>
> So printing a numeric value is quite useless when users actually run
> into these errors -- which is why I like err() so much. How about using
> strerror() instead?
>
Yes. using perror() does give better information. There are other
places in this file that use errno. How about I make that a separate
patch and catch all of them at once to use perror() as a follow-up
change? That way I fix all at once without adding more changes to
this patch.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH 10/17] mm: rmap preparation for remap_anon_pages
From: Linus Torvalds @ 2014-10-03 18:31 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: qemu-devel, KVM list, Linux Kernel Mailing List, linux-mm,
Linux API, Andres Lagar-Cavilla, Dave Hansen, Paolo Bonzini,
Rik van Riel, Mel Gorman, Andy Lutomirski, Andrew Morton,
Sasha Levin, Hugh Dickins, Peter Feiner, \Dr. David Alan Gilbert\,
Christopher Covington, Johannes Weiner, Android Kernel Team,
Robert Love, Dmitry Adamushko, Neil Brown, Mike Hommey,
Taras Glek <tg>
In-Reply-To: <1412356087-16115-11-git-send-email-aarcange@redhat.com>
On Fri, Oct 3, 2014 at 10:08 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
>
> Overall this looks a fairly small change to the rmap code, notably
> less intrusive than the nonlinear vmas created by remap_file_pages.
Considering that remap_file_pages() was an unmitigated disaster, and
-mm has a patch to remove it entirely, I'm not at all convinced this
is a good argument.
We thought remap_file_pages() was a good idea, and it really really
really wasn't. Almost nobody used it, why would the anonymous page
case be any different?
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 04/17] mm: gup: make get_user_pages_fast and __get_user_pages_fast latency conscious
From: Linus Torvalds @ 2014-10-03 18:23 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: qemu-devel, KVM list, Linux Kernel Mailing List, linux-mm,
Linux API, Andres Lagar-Cavilla, Dave Hansen, Paolo Bonzini,
Rik van Riel, Mel Gorman, Andy Lutomirski, Andrew Morton,
Sasha Levin, Hugh Dickins, Peter Feiner, \Dr. David Alan Gilbert\,
Christopher Covington, Johannes Weiner, Android Kernel Team,
Robert Love, Dmitry Adamushko, Neil Brown, Mike Hommey,
Taras Glek <tg>
In-Reply-To: <1412356087-16115-5-git-send-email-aarcange@redhat.com>
On Fri, Oct 3, 2014 at 10:07 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> This teaches gup_fast and __gup_fast to re-enable irqs and
> cond_resched() if possible every BATCH_PAGES.
This is disgusting.
Many (most?) __gup_fast() users just want a single page, and the
stupid overhead of the multi-page version is already unnecessary.
This just makes things much worse.
Quite frankly, we should make a single-page version of __gup_fast(),
and convert existign users to use that. After that, the few multi-page
users could have this extra latency control stuff.
And yes, the single-page version of get_user_pages_fast() is actually
latency-critical. shared futexes hit it hard, and yes, I've seen this
in profiles.
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 01/17] mm: gup: add FOLL_TRIED
From: Linus Torvalds @ 2014-10-03 18:15 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: qemu-devel, KVM list, Linux Kernel Mailing List, linux-mm,
Linux API, Andres Lagar-Cavilla, Dave Hansen, Paolo Bonzini,
Rik van Riel, Mel Gorman, Andy Lutomirski, Andrew Morton,
Sasha Levin, Hugh Dickins, Peter Feiner, \Dr. David Alan Gilbert\,
Christopher Covington, Johannes Weiner, Android Kernel Team,
Robert Love, Dmitry Adamushko, Neil Brown, Mike Hommey,
Taras Glek <tg>
In-Reply-To: <1412356087-16115-2-git-send-email-aarcange@redhat.com>
This needs more explanation than that one-liner comment. Make the
commit message explain why the new FOLL_TRIED flag exists.
Linus
On Fri, Oct 3, 2014 at 10:07 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> From: Andres Lagar-Cavilla <andreslc@google.com>
>
> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Andres Lagar-Cavilla <andreslc@google.com>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86,seccomp,prctl: Remove PR_TSC_SIGSEGV and seccomp TSC filtering
From: Andy Lutomirski @ 2014-10-03 17:59 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Paul Mackerras, Arnaldo Carvalho de Melo, X86 ML,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Peter Zijlstra, Ingo Molnar, Kees Cook, Erik Bosman,
H. Peter Anvin, Linux API, Michael Kerrisk-manpages
In-Reply-To: <20141003174141.GR2342-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[cc's re-added]
On Fri, Oct 3, 2014 at 10:41 AM, Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
> On Fri, Oct 03, 2014 at 10:18:14AM -0700, Andy Lutomirski wrote:
>> Weakness 2: On most configurations, most or all userspace processes
>> have unrestricted access to RDPMC, which is even better than RDTSC
>> for exploiting timing attacks.
>
> "User access of the RDPMC instruction is not guaranteed. Like RDTSC,
> user access is controlled by a bit in CR4. CR4.PCE (bit-8) controls
> whether or not a user program can execute the RDPMC instruction
> without faulting"
>
> I don't think there's was a seccomp leak of RDPMC because of this, the
> rdtsc and rdpmc seems to be linked to the same cr4 tweak.
RDPMC is controlled by CR4.PCE, whereas RDTSC is controlled by
CR4.TSD, so, unless there's magic that I'm unaware of, seccomp never
blocked RDPMC access. I don't know whether circa-2008 kernels set PCE
(or perhaps left it set if BIOS set it) at boot, but certainly almost
everyone on any kernel for the last few years has RDPMC enabled in
ring 3.
>
> The vsyscall data was leaked right, but you can't compare the
> two. Sure it's better to block that too but it's not comparable to
> give tsc access to apps running under seccomp.
>
> The time of the day isn't secret either (ok it could be an issue if
> you intend to run the system on some secret time in the past or future
> but this sounds not a practical issue).
>
> What's not public info and should never be leaked to seccomp
> sandboxes, is the tsc at that kind of cycle count granularity (and the
> various gettimeofday variants with nanosecond granularity). I thought
> RDPMC was blocked too with the same CR4 tweak... if that wasn't the
> case and you could get tsc granular information into a seccomp
> sandbox, that's not ok because it allows for covert channel attacks.
The HPET very fine granularity. It's slow to access, but that isn't
necessarily a problem for attackers.
I agree that this is problematic, and I want to fix it. The trouble
is that I'm not sure it's fixable in a sane manner with the current
semantics. CR4.PCE, in particular, will need to be a function of both
per-thread state (PR_TSC_SIGSEGV) and per-mm state (whether perf_event
self-monitoring is on). Getting the context switching logic correct
without hurting the common case too much will be quite complicated.
On top of this, supporting something like PR_TSC_SIGSEGV per-thread
using seccomp mode 2 should really be done by redirecting vdso-based
timing to use syscalls, and that's fundamentally per mm.
Hence my proposal of removing the current insecure model so that
adding a secure variant will be straightforward, rather than trying to
shoehorn a fix on top of the current ABI.
The eventual fix could still disable the TSC in a strict seccomp task
by default if the task is single threaded. Yes, that won't quite
cover old Chromium-style sandboxes, but those are rapidly being
replaced by new designs using seccomp mode 2 anyway.
Also, keep in mind that multithreaded attackers can exploit timing
attacks without hardware help at all: one thread can just run a timing
loop.
--Andy
^ 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