linux-kernel - Re: [PATCH 3/5] sched: Detect call to schedule from critical entry code

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]

Message-ID: <20201007093436.GG3165@suse.de>
Date:   Wed, 7 Oct 2020 10:34:36 +0100
From:   Mel Gorman <mgorman@...e.de>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Phil Auld <pauld@...hat.com>,
        Marcelo Tosatti <mtosatti@...hat.com>
Subject: Re: [PATCH 3/5] sched: Detect call to schedule from critical entry
 code

On Mon, Oct 05, 2020 at 02:26:48PM +0200, Frederic Weisbecker wrote:
> On Mon, Oct 05, 2020 at 01:23:53PM +0200, Peter Zijlstra wrote:
> > On Mon, Oct 05, 2020 at 12:49:17PM +0200, Frederic Weisbecker wrote:
> > > Detect calls to schedule() between user_enter() and user_exit(). Those
> > > are symptoms of early entry code that either forgot to protect a call
> > > to schedule() inside exception_enter()/exception_exit() or, in the case
> > > of HAVE_CONTEXT_TRACKING_OFFSTACK, enabled interrupts or preemption in
> > > a wrong spot.
> > > 
> > > Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> > > Cc: Marcelo Tosatti <mtosatti@...hat.com>
> > > Cc: Paul E. McKenney <paulmck@...nel.org>
> > > Cc: Peter Zijlstra <peterz@...radead.org>
> > > Cc: Phil Auld <pauld@...hat.com>
> > > Cc: Thomas Gleixner <tglx@...utronix.de>
> > > ---
> > >  kernel/sched/core.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > index 2d95dc3f4644..d31a79e073e3 100644
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -4295,6 +4295,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
> > >  		preempt_count_set(PREEMPT_DISABLED);
> > >  	}
> > >  	rcu_sleep_check();
> > > +	WARN_ON_ONCE(ct_state() == CONTEXT_USER);
> > 
> > 	SCHED_WARN_ON() ?
> 
> Bah! That's exactly what I was looking for.
> 
> > No point in unconditionally polluting that path. Although, per MeL, we
> > should probably invest in CONFIG_SCHED_DEBUG_I_MEANS_IT :/
> 
> Because CONFIG_SCHED_DEBUG is often used by default on distros?
> 

SCHED_DEBUG is generally useful (e.g. figuring out weird topology problems
on new hardware). The overhead isn't too bad when schedstats are
disabled so it would be nice to avoid adding too much overhead via
SCHED_DEBUG.

Other debugging options -- not so much. A lot of them are useful for
development but there are people who request them be enabled anyway
thinking that they improve security somehow when in reality they might,
at best, detect a hardware issue that happens to hit a specific structure.

-- 
Mel Gorman
SUSE Labs