linux-kernel - Re: [PATCH v3 2/2] Documentation: core-api: entry: Add comments about nesting

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] [day] [month] [year] [list]

Message-ID: <20211217175644.GZ641268@paulmck-ThinkPad-P17-Gen-1>
Date:   Fri, 17 Dec 2021 09:56:44 -0800
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Nicolas Saenz Julienne <nsaenzju@...hat.com>
Cc:     tglx@...utronix.de, mark.rutland@....com, rostedt@...dmis.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        rcu@...r.kernel.org, peterz@...radead.org, mtosatti@...hat.com,
        frederic@...nel.org, corbet@....net
Subject: Re: [PATCH v3 2/2] Documentation: core-api: entry: Add comments
 about nesting

On Fri, Dec 17, 2021 at 11:57:53AM +0100, Nicolas Saenz Julienne wrote:
> The topic of nesting and reentrancy in the context of early entry code
> hasn't been addressed so far. So do it.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@...hat.com>

Looks good!  Just a few small suggestions below.

							Thanx, Paul

> ---
> 
> NOTE: I moved this into a separate patch to simplify the review.
> 
>  Documentation/core-api/entry.rst | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/core-api/entry.rst b/Documentation/core-api/entry.rst
> index 3f80537f2826..f665f201ead0 100644
> --- a/Documentation/core-api/entry.rst
> +++ b/Documentation/core-api/entry.rst
> @@ -105,6 +105,8 @@ has to do extra work between the various steps. In such cases it has to
>  ensure that enter_from_user_mode() is called first on entry and
>  exit_to_user_mode() is called last on exit.
>  
> +Syscalls shouldn't nest. If it were to happen, RCU / context tracking will
> +catch the misbehavior and print out a warning.

How about like this?

Do not nest syscalls.  Nested systcalls will cause RCU and/or context
tracking to print a warning.

>  KVM
>  ---
> @@ -121,6 +123,9 @@ Task work handling is done separately for guest at the boundary of the
>  vcpu_run() loop via xfer_to_guest_mode_handle_work() which is a subset of
>  the work handled on return to user space.
>  
> +Nesting doesn't make sense in the context of KVM entry/exit transitions, it
> +shouldn't happen.

Like this?

Do not nest KVM entry/exit transitions because doing so is nonsensical.

>  Interrupts and regular exceptions
>  ---------------------------------
>  
> @@ -180,6 +185,16 @@ before it handles soft interrupts, whose handlers must run in BH context rather
>  than irq-disabled context. In addition, irqentry_exit() might schedule, which
>  also requires that HARDIRQ_OFFSET has been removed from the preemption count.
>  
> +Even though interrupt handlers are expected to run with local interrupts
> +disabled, interrupt nesting is common from an entry/exit perspective. For
> +example, softirq handling happens within an irqentry_{enter,exit}() block, with

The last comma on the above line needs to be removed, so just a space
character between "block" and "with".

> +local interrupts enabled. Also, although uncommon, nothing prevents an
> +interrupt handler from re-enabling interrupts.
> +
> +Interrupt entry/exit code doesn't strictly need to handle reentrancy, since it
> +runs with local interrupts disabled. But NMIs can happen anytime, and a lot of
> +the entry code is shared between the two.
> +
>  NMI and NMI-like exceptions
>  ---------------------------
>  
> @@ -259,3 +274,7 @@ and for e.g. a debug exception it can look like this:
>  
>  There is no combined irqentry_nmi_if_kernel() function available as the
>  above cannot be handled in an exception-agnostic way.
> +
> +NMIs can happen in any context. For example, an NMI-like exception triggered
> +while handling an NMI. So NMI entry code has to be reentrant and state updates
> +need to handle nesting.
> -- 
> 2.33.1
>