On Mon, Feb 12, 2018 at 4:04 PM, Dmitry Vyukov <dvyu...@google.com> 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, >> }; > > > Another perf deadlock that involves splice: > https://groups.google.com/d/msg/syzkaller-bugs/vVy6Zj3wPxo/6oj5U6WiAwAJ
Peter, What's the status of this? Is your patch still relevant?