linux-kernel - Re: [RFC PATCH 1/5] entry: Pass pt_regs to irqentry_exit_cond

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:   Mon, 8 Aug 2022 10:34:19 -0700
From:   Ira Weiny <ira.weiny@...el.com>
To:     Borislav Petkov <bp@...en8.de>
CC:     Rik van Riel <riel@...riel.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
        <linux-kernel@...r.kernel.org>, <kernel-team@...com>
Subject: Re: [RFC PATCH 1/5] entry: Pass pt_regs to
 irqentry_exit_cond_resched()

On Mon, Aug 08, 2022 at 12:38:08PM +0200, Borislav Petkov wrote:
> On Fri, Aug 05, 2022 at 10:30:05AM -0700, ira.weiny@...el.com wrote:
> > From: Ira Weiny <ira.weiny@...el.com>
> > 
> > Auxiliary pt_regs space needs to be manipulated by the generic
> > entry/exit code.
> > 
> > Ideally irqentry_exit() would take care of handling any auxiliary
> > pt_regs on exit.  Unfortunately, irqentry_exit() is not the only exit
> > from exception path.  The call to irqentry_exit_cond_resched() from
> > xen_pv_evtchn_do_upcall() bypasses irqentry_exit().
> > 
> > Make irqentry_exit_cond_resched() symmetrical with irqentry_enter() by
> > passing pt_regs to it.  This makes irqentry_exit_cond_resched() capable
> > of handling auxiliary pt_regs in future patches.
> > 
> > Cc: Rik van Riel <riel@...riel.com>
> > Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> > Cc: Borislav Petkov <bp@...en8.de>
> > Signed-off-by: Ira Weiny <ira.weiny@...el.com>
> > 
> > ---
> > Forward ported from PKS series:
> > 	https://lore.kernel.org/lkml/20220419170649.1022246-19-ira.weiny@intel.com/
> > ---
> >  arch/arm64/include/asm/preempt.h |  2 +-
> >  arch/arm64/kernel/entry-common.c |  4 ++--
> >  arch/x86/entry/common.c          |  2 +-
> >  include/linux/entry-common.h     | 17 ++++++++------
> >  kernel/entry/common.c            | 13 +++++++----
> >  kernel/sched/core.c              | 40 ++++++++++++++++----------------
> >  6 files changed, 43 insertions(+), 35 deletions(-)
> 
> Why all this churn?
> 
> Why can't you add a parameter to irqentry_exit():
> 
>   noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state, bool cond_resched);

I thought about that but generally have been steered away from using bool
arguments like this.

> 
> and then have all callers except xen_pv_evtchn_do_upcall() pass in false
> and this way have all exit paths end up in irqentry_exit()?
> 
> And, ofc, move the true case which is the body of
> raw_irqentry_exit_cond_resched() to irqentry_exit() and then get rid of
> former.

Looking this over I think this may be a reasonable use of a flag like that.

I'm not sure if this series is going to land given the discussion of
performance.  Would this clean up be welcome on its own?  I'm willing to respin
this patch and submit if so.

Ira

> 
> xen_pv_evtchn_do_upcall() will, ofc, do:
> 
>         if (inhcall && !WARN_ON_ONCE(state.exit_rcu)) {
>                 irqentry_exit(regs, state, true);
>                 instrumentation_end();
>                 restore_inhcall(inhcall);
>         } else {
>                 instrumentation_end();
>                 irqentry_exit(regs, state, false);
> 
> Hmmm?
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette