On Fri, Oct 27, 2017 at 05:11:37PM +0200, Peter Zijlstra wrote:
> On Fri, Oct 27, 2017 at 01:30:30AM -0700, syzbot wrote:
>
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 4.13.0-next-20170911+ #19 Not tainted
> > ------------------------------------------------------
> > syz-executor2/12380 is trying to acquire lock:
> > (&ctx->mutex){+.+.}, at: [<ffffffff8180923c>]
> > perf_event_ctx_lock_nested+0x1dc/0x3c0 kernel/events/core.c:1210
> >
> > but task is already holding lock:
> > (&pipe->mutex/1){+.+.}, at: [<ffffffff81ac0fa6>] pipe_lock_nested
> > fs/pipe.c:66 [inline]
> > (&pipe->mutex/1){+.+.}, at: [<ffffffff81ac0fa6>] pipe_lock+0x56/0x70
> > fs/pipe.c:74
> >
> > which lock already depends on the new lock.
>
>
> ARRGH!!
>
> that translates like the below, which is an absolute maze and requires
> at least 5 concurrent callstacks, possibly more.
>
> We already had a lot of fun with hotplug-perf-ftrace, but the below
> contains more. Let me try and page that previous crap back.
>
>
>
> perf_ioctl()
> #0 perf_event_ctx_lock() [ctx->mutex]
> perf_event_set_filter
> #1 ftrace_profile_set_filter [event_mutex]
>
>
>
>
> sys_perf_event_open
> ...
> perf_trace_init
> #1 mutex_lock [event_mutex]
> trace_event_reg
> tracepoint_probe_register
> #2 mutex_lock() [tracepoints_mutex]
> tracepoint_add_func()
> #3 static_key_slow_inc() [cpuhotplug_lock]
>
>
>
>
>
> cpuhp_setup_state_nocalls
> #3 cpus_read_lock [cpuhotplug_lock]
> __cpuhp_setup_state_cpuslocked
> #4 mutex_lock [cpuhp_state_mutex]
> cpuhp_issue_call
> #5 cpuhp_invoke_ap_callback() [cpuhp_state]
>
>
> #5 cpuhp_invoke_callback [cpuhp_state]
> ...
> devtmpfs_create_node
> #6 wait_for_completion() [&req.done]
>
> devtmpfsd
> handle_create
> #7 filename_create [sb_writers]
> #6 complete [&req.done]
>
>
>
>
>
>
> do_splice
> #7 file_start_write() [sb_writers]
> do_splice_from
> iter_file_splice_write
> #8 pipe_lock [pipe->mutex]
>
>
>
>
>
> do_splice
> #8 pipe_lock [pipe->mutex]
> do_splice_to
> ...
> #0 perf_read() [ctx->mutex]
>
So arguably that last op, splice_read from a perf fd is fairly
pointless and we could dis-allow that. How about something like the
below?
---
kernel/events/core.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 04989fb769f0..fd03f3082ee3 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5468,6 +5468,13 @@ static int perf_fasync(int fd, struct file *filp, int on)
return 0;
}
+static ssize_t perf_splice_read(struct file *file, loff_t *ppos,
+ struct pipe_inode_info *pope, size_t len,
+ unsigned int flags)
+{
+ return -EOPNOTSUPP;
+}
+
static const struct file_operations perf_fops = {
.llseek = no_llseek,
.release = perf_release,
@@ -5477,6 +5484,7 @@ static int perf_fasync(int fd, struct file *filp, int on)
.compat_ioctl = perf_compat_ioctl,
.mmap = perf_mmap,
.fasync = perf_fasync,
+ .splice_read = perf_splice_read,
};
/*
