linux-kernel - Re: [RFC] rcu: use killable versions of swait

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170615155039.GP27288@wotan.suse.de>
Date:   Thu, 15 Jun 2017 17:50:39 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:     "Luis R. Rodriguez" <mcgrof@...nel.org>, josh@...htriplett.org,
        rostedt@...dmis.org, mathieu.desnoyers@...icios.com,
        jiangshanlai@...il.com, paul.gortmaker@...driver.com,
        ebiederm@...ssion.com, dmitry.torokhov@...il.com,
        linux-kernel@...r.kernel.org, oleg@...hat.com
Subject: Re: [RFC] rcu: use killable versions of swait

On Wed, Jun 14, 2017 at 04:43:45PM -0700, Paul E. McKenney wrote:
> On Wed, Jun 14, 2017 at 04:06:39PM -0700, Luis R. Rodriguez wrote:
> > These waits don't even check for the return value for interruption,
> > using the non-killable variants means we could be killed by other
> > signals than SIGKILL, this is fragile.
> 
> A number of people asserted that kthreads could never catch signals.
> I checked this at the time, and it seemed like this was the case, and
> the call to ignore_signals() seems to confirm this.
> 
> So it looks to me like RCU doesn't care about this change (give or
> take any affect on the load average, anyway), but there is much that I
> don't know about Linux-kernel signal handling, so please feel free to
> educate me.

Thanks, I had seen the kthread but figured best to ask, just got into parnaoia
mode. If we were to do a sanity check for usage we'd then have to white list
when kthreads are used, however since we don't care to be interrupted why not
use a wait which is also explicit about our current uninterruptible state?

> > Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
> > ---
> > 
> > The killable swaits were just posted [1] as part of a series where SIGCHLD
> > was detected as interrupting and killing kernel calls waiting using
> > non-killable swaits [1]. The fragility here made curious about other callers
> > and seeing if they really meant to use such broad wait which captures a lot
> > of signals.
> > 
> > I can't see why we'd want to have these killed by other signals, specialy
> > since it seems we don't even check for the return value... Granted to abort
> > properly we'd have to check for the return value for -ERESTARTSYS, but yeah,
> > none of this is done, so it would seem we don't want fragile signals
> > interrupting these ?
> 
> The later WARN_ON(signal_pending(current)) complains if a signal somehow
> makes it to this task.  Assuming that the signal is nonfatal, anyway.

I see, how about just using swait_event_timeout() and removing the WARN_ON()?
Is there a reason for having the interruptible ?

> > Also can someone confirm if the original change of to swait_event_timeout()
> > from wait_event_interruptible_timeout() was actually intentional on
> > synchronize_sched_expedited_wait() on commit abedf8e2419fb ("rcu: Use simple
> > wait queues where possible in rcutree") ? I can't easily confirm.
> 
> This is also called from a workqueue (at least once the core_initcall()s
> get done), which again should mean no signals.  A WARN_ON(ret < 0)
> complains if this ever changes.  So it should be OK that this is
> swait_event_timeout().  And expedited grace periods are supposed to
> get done quickly, so effect on the load average should be negligible.

Great thanks.

> Or am I missing something?

No, just got into parnoia mode and better to ask than be sorry later!

  Luis