linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Wen Yang <wen.yang99@zte.com.cn>, majiang <ma.jiang@zte.com.cn>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	Andrey Vagin <avagin@virtuozzo.com>,
	Cyrill Gorcunov <gorcunov@gmail.com>
Subject: Re: [PATCH] signal: Don't send signals to tasks that don't exist
Date: Fri, 17 Aug 2018 13:46:36 -0500	[thread overview]
Message-ID: <87pnyg7rw3.fsf@xmission.com> (raw)
In-Reply-To: <CACT4Y+bYFatW=CMgH4M6AjtjO297gbQ8xwnEV0_f7J64FLQ4QQ@mail.gmail.com> (Dmitry Vyukov's message of "Fri, 17 Aug 2018 10:34:12 -0700")

Dmitry Vyukov <dvyukov@google.com> writes:

> On Wed, Aug 15, 2018 at 9:04 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>>
>> Recently syzbot reported crashes in send_sigio_to_task and
>> send_sigurg_to_task in linux-next.  Despite finding a reproducer
>> syzbot apparently did not bisected this or otherwise track down the
>> offending commit in linux-next.
>>
>> I happened to see this report and examined the code because I had
>> recently changed these functions as part of making PIDTYPE_TGID a real
>> pid type so that fork would does not need to restart when receiving a
>> signal.  By examination I see that I spotted a bug in the code
>> that could explain the reported crashes.
>>
>> When I took Oleg's suggestion and optimized send_sigurg and send_sigio
>> to only send to a single task when type is PIDTYPE_PID or PIDTYPE_TGID
>> I failed to handle pids that no longer point to tasks.  The macro
>> do_each_pid_task simply iterates for zero iterations.  With pid_task
>> an explicit NULL test is needed.
>>
>> Update the code to include the missing NULL test.
>>
>> Fixes: 019191342fec ("signal: Use PIDTYPE_TGID to clearly store where file signals will be sent")
>> Reported-by: syzkaller-bugs@googlegroups.com
>
> Since the commit does not contain the syzbot-provided Reported-by tag,
> we need to tell syzbot that this is fixed explicitly:

Nor will my commits ever contain that information.  That is information
only of use to syzbot.  That is not information useful to anyone else.

Further syzbot did not track this down and report this.  Syzbot said
something is fishy here and happened to CC a public mailing list.  Only
by chance did I see the report.  There was enough information to start
an investigation but it certainly was not any kind of useful bug report.

It is very annoying that despite syzbot claming to have a reproducer
syzbot completely failed to locate the problem commit or the proper
people to repor the issue to.  I looked at the syzbot website link and
there was no evidence that syzbot even tried to track down which branch
in linux-next the commit came from.  Much less to identify the commit on
that branch.

Very annoyingly syzbot sent out emails and report this before it found a
reproducer.  This is despite several people explicitly asking syzbot
to not report issuing on linux-next where syzbot does not have a
reproducer and it can not track down the offending commit.

> #syz fix: signal: Don't send signals to tasks that don't exist

Private internal communication on a public mailing list is rude.  Please
cut it out.

I appreciate the eagerness to report bugs.  But to play well with others
and not waste developers valuable time syzbot needs to track down the
offending commits and track fixes tags like everyone else.  Special
magic syzbot tags are annoying noise.

I will give credit where credit is due.  But syzbot is not so valuable
it can set rules for everyone else.  Automation is valuable when it
removes work.  Syzbot is not doing a good job at making the most of
developers limited time.

Cyrill Gorconov and Andrey Vagin did a much better job in tracking this
down and reporting this.  They just took a little bit longer.  Please
look at what they sent if you need an example of a useful bug report
looks like.

Eric

  reply	other threads:[~2018-08-17 18:46 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bug-200447-5873-b2kAsSyE1X@https.bugzilla.kernel.org/>
     [not found] ` <CA+55aFyOaEGc_wjRuAxYZH7D4zi4xUQTqUwMmUzxFJQ__2pXuQ@mail.gmail.com>
     [not found]   ` <87h8l9p7bg.fsf@xmission.com>
     [not found]     ` <20180709104158.GA23796@redhat.com>
     [not found]       ` <87sh4so5jv.fsf@xmission.com>
     [not found]         ` <20180709145726.GA26149@redhat.com>
     [not found]           ` <877em4nxo0.fsf@xmission.com>
     [not found]             ` <CA+55aFwq90_KeRUesktap7L_4Hp3gatKZ28RYw1jXBYeOqUoeA@mail.gmail.com>
     [not found]               ` <87lgakm4ol.fsf@xmission.com>
     [not found]                 ` <CA+55aFz1XFvOgJySKVNQD9VS4hys0J7rozxqd3s5ND0z80tfVw@mail.gmail.com>
     [not found]                   ` <20180710134639.GA2453@redhat.com>
2018-07-10 16:00                     ` [Bug 200447] infinite loop in fork syscall Eric W. Biederman
2018-07-11 12:08                       ` Oleg Nesterov
     [not found]                     ` <CA+55aFxcjSYAj-CZFEuDwiwZyMg+prV_jeP_Vuh06RJA0BboSw@mail.gmail.com>
2018-07-11  2:41                       ` [RFC][PATCH 0/11] PIDTYPE_TGID and fewer fork restarts Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 01/11] pids: Initialize leader_pid in init_task Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 02/11] pids: Move task_pid_type into sched/signal.h Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 03/11] pids: Compute task_tgid using signal->leader_pid Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 04/11] kvm: Don't open code task_pid in kvm_vcpu_ioctl Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 05/11] pids: Move the pgrp and session pid pointers from task_struct to signal_struct Eric W. Biederman
2018-07-17 11:59                           ` Oleg Nesterov
2018-07-11  2:44                         ` [RFC][PATCH 06/11] pid: Implement PIDTYPE_TGID Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 07/11] signal: Deliver group signals via PIDTYPE_TGID not PIDTYPE_PID Eric W. Biederman
2018-07-16 12:51                           ` Oleg Nesterov
2018-07-16 14:50                             ` Eric W. Biederman
2018-07-16 17:17                               ` Linus Torvalds
2018-07-16 18:01                                 ` Eric W. Biederman
2018-07-16 18:40                                   ` Linus Torvalds
2018-07-17  9:56                                   ` Oleg Nesterov
2018-07-17 10:18                                     ` Oleg Nesterov
2018-07-20 23:41                                       ` Eric W. Biederman
2018-07-17 16:38                               ` Linus Torvalds
2018-07-20 23:27                                 ` Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 08/11] signal: Use PIDTYPE_TGID to clearly store where file signals will be sent Eric W. Biederman
2018-07-11  2:44                         ` [RFC][PATCH 09/11] tty_io: Use do_send_sig_info in __do_SACK to forcibly kill tasks Eric W. Biederman
2018-07-16 14:55                           ` Oleg Nesterov
2018-07-16 15:08                             ` Eric W. Biederman
2018-07-16 16:50                               ` Linus Torvalds
2018-07-16 19:17                                 ` Eric W. Biederman
2018-07-16 19:36                                   ` Linus Torvalds
2018-07-17  1:48                                     ` Eric W. Biederman
2018-07-17 10:58                               ` Oleg Nesterov
2018-07-11  2:44                         ` [RFC][PATCH 10/11] signal: Push pid type from signal senders down into __send_signal Eric W. Biederman
2018-07-11  3:11                           ` Linus Torvalds
2018-07-11  2:44                         ` [RFC][PATCH 11/11] signal: Ignore all but multi-process signals that come in during fork Eric W. Biederman
2018-07-11 14:14                           ` Oleg Nesterov
2018-07-11 16:02                             ` Eric W. Biederman
2018-07-12 13:42                               ` Oleg Nesterov
2018-07-12 17:11                                 ` Eric W. Biederman
2018-07-13 14:51                             ` Eric W. Biederman
2018-07-24  3:22                         ` [PATCH 00/20] PIDTYPE_TGID removal of fork restarts Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 01/20] pids: Initialize leader_pid in init_task Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 02/20] pids: Move task_pid_type into sched/signal.h Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 03/20] pids: Compute task_tgid using signal->leader_pid Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 04/20] kvm: Don't open code task_pid in kvm_vcpu_ioctl Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 05/20] pids: Move the pgrp and session pid pointers from task_struct to signal_struct Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 06/20] pid: Implement PIDTYPE_TGID Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 07/20] signal: Use PIDTYPE_TGID to clearly store where file signals will be sent Eric W. Biederman
2018-08-16  4:04                             ` [PATCH] signal: Don't send signals to tasks that don't exist Eric W. Biederman
2018-08-17 17:34                               ` Dmitry Vyukov
2018-08-17 18:46                                 ` Eric W. Biederman [this message]
2018-08-17 19:24                                   ` Andrew Morton
2018-08-18  5:51                                     ` Eric W. Biederman
2018-08-20 19:26                                       ` J. Bruce Fields
2018-07-24  3:24                           ` [PATCH 08/20] posix-timers: Noralize good_sigevent Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 09/20] signal: Pass pid and pid type into send_sigqueue Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 10/20] signal: Pass pid type into group_send_sig_info Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 11/20] signal: Pass pid type into send_sigio_to_task & send_sigurg_to_task Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 12/20] signal: Pass pid type into do_send_sig_info Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 13/20] signal: Push pid type down into send_signal Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 14/20] signal: Push pid type down into __send_signal Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 15/20] signal: Push pid type down into complete_signal Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 16/20] fork: Move and describe why the code examines PIDNS_ADDING Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 17/20] fork: Unconditionally exit if a fatal signal is pending Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 18/20] signal: Add calculate_sigpending() Eric W. Biederman
2018-07-26 13:20                             ` Oleg Nesterov
2018-07-26 15:13                               ` Eric W. Biederman
2018-07-26 16:24                                 ` Oleg Nesterov
2018-07-24  3:24                           ` [PATCH 19/20] fork: Have new threads join on-going signal group stops Eric W. Biederman
2018-07-24  3:24                           ` [PATCH 20/20] signal: Don't restart fork when signals come in Eric W. Biederman
2018-07-24 17:27                             ` Linus Torvalds
2018-07-24 17:58                               ` Eric W. Biederman
2018-07-24 18:29                                 ` Linus Torvalds
2018-07-24 20:05                                   ` Eric W. Biederman
2018-07-24 20:15                                     ` Linus Torvalds
2018-07-24 20:40                                       ` [PATCH v2 " Eric W. Biederman
2018-07-24 20:56                                         ` Linus Torvalds
2018-07-24 21:24                                           ` Eric W. Biederman
2018-07-25  3:56                                             ` [PATCH v3 " Eric W. Biederman
2018-07-26 13:41                                               ` Oleg Nesterov
2018-07-26 14:42                                                 ` Eric W. Biederman
2018-07-26 15:55                                                   ` Oleg Nesterov
2018-08-09  6:19                                                     ` Eric W. Biederman
2018-07-26 16:21                                               ` Oleg Nesterov
2018-07-24 17:29                           ` [PATCH 00/20] PIDTYPE_TGID removal of fork restarts Linus Torvalds
2018-07-25 16:05                           ` Oleg Nesterov
2018-07-25 16:58                             ` Eric W. Biederman
2018-08-09  6:53                           ` [PATCH v5 0/6] Not restarting for due to signals Eric W. Biederman
2018-08-09  6:56                             ` [PATCH v5 1/6] fork: Move and describe why the code examines PIDNS_ADDING Eric W. Biederman
2018-08-09  6:56                             ` [PATCH v5 2/6] fork: Unconditionally exit if a fatal signal is pending Eric W. Biederman
2018-08-09  6:56                             ` [PATCH v5 3/6] signal: Add calculate_sigpending() Eric W. Biederman
2018-08-09  6:56                             ` [PATCH v5 4/6] fork: Skip setting TIF_SIGPENDING in ptrace_init_task Eric W. Biederman
2018-08-09  6:56                             ` [PATCH v5 5/6] fork: Have new threads join on-going signal group stops Eric W. Biederman
2018-08-09  6:56                             ` [PATCH v5 6/6] signal: Don't restart fork when signals come in Eric W. Biederman
2018-08-09 17:15                               ` Linus Torvalds
2018-08-09 17:42                                 ` Eric W. Biederman
2018-08-09 18:09                                 ` [PATCH v6 " Eric W. Biederman
2018-08-09 17:16                             ` [PATCH v5 0/6] Not restarting for due to signals Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pnyg7rw3.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@virtuozzo.com \
    --cc=bfields@fieldses.org \
    --cc=dvyukov@google.com \
    --cc=gorcunov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ma.jiang@zte.com.cn \
    --cc=oleg@redhat.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=torvalds@linux-foundation.org \
    --cc=wen.yang99@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).