From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org, Wen Yang <wen.yang99@zte.com.cn>,
majiang <ma.jiang@zte.com.cn>
Subject: Re: [RFC][PATCH 11/11] signal: Ignore all but multi-process signals that come in during fork.
Date: Wed, 11 Jul 2018 16:14:56 +0200 [thread overview]
Message-ID: <20180711141456.GA6636@redhat.com> (raw)
In-Reply-To: <20180711024459.10654-11-ebiederm@xmission.com>
On 07/10, Eric W. Biederman wrote:
>
> @@ -1602,6 +1603,20 @@ static __latent_entropy struct task_struct *copy_process(
> {
> int retval;
> struct task_struct *p;
> + unsigned seq;
> +
> + /*
> + * Signals that are delivered to multiple processes need to be
> + * delivered to just the parent before the fork or both the
> + * parent and the child after the fork. Cache the multiple
> + * process signal sequence number so we can detect any of
> + * these signals that happen during the fork. In the unlikely
> + * event a signal comes in while fork is starting and restart
> + * fork to handle the signal.
> + */
> + seq = read_seqcount_begin(¤t->signal->multi_process_seq);
> + if (signal_pending(current))
> + return ERR_PTR(-ERESTARTNOINTR);
>
> /*
> * Don't allow sharing the root directory with processes in a different
> @@ -1930,8 +1945,8 @@ static __latent_entropy struct task_struct *copy_process(
> * A fatal signal pending means that current will exit, so the new
> * thread can't slip out of an OOM kill (or normal SIGKILL).
> */
> - recalc_sigpending();
> - if (signal_pending(current)) {
> + if (read_seqcount_retry(¤t->signal->multi_process_seq, seq) ||
> + fatal_signal_pending(current)) {
> retval = -ERESTARTNOINTR;
> goto bad_fork_cancel_cgroup;
So once again, I think this is not right, see the discussion on bugzilla.
If signal_pending() == T we simply can't know if copy_process() can succeed or not.
I have already mentioned the races with stop/freeze, but I think there are more.
And in fact I think that the fact that signal_wake_up() helps to avoid the races
with fork() is useful. Say, we could add signal_wake_up() into syscall_regfunc()
and kill syscall_tracepoint_update(). Not that I think this particular change makes
any sense, but it can work.
That is why I tried to sugest another approach. copy_process() should always fail
if signal_pending() == T, just the "real" signal should not disturb the forking
thread unless the signal is fatal or multi-process.
This also makes another difference in multi-threaded case, a signal with a handler
sent to a forking process will be re-targeted to another thread which can handle it;
with your patch this signal will be "blocked" until fork() finishes or until another
thread gets TIF_SIGPENDING. Not that I think this is that important, but still.
Oleg.
next prev parent reply other threads:[~2018-07-11 14:15 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 [this message]
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
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=20180711141456.GA6636@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ma.jiang@zte.com.cn \
--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).