linux-kernel - Re: [PATCH] signal: Don't send signals to tasks that don't exist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Aug 2018 13:46:36 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Wen Yang <wen.yang99@....com.cn>,
        majiang <ma.jiang@....com.cn>,
        "J. Bruce Fields" <bfields@...ldses.org>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Andrey Vagin <avagin@...tuozzo.com>,
        Cyrill Gorcunov <gorcunov@...il.com>
Subject: Re: [PATCH] signal: Don't send signals to tasks that don't exist

Dmitry Vyukov <dvyukov@...gle.com> writes:

> On Wed, Aug 15, 2018 at 9:04 PM, Eric W. Biederman
> <ebiederm@...ssion.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@...glegroups.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