Re: [linux-safety] [RFC PATCH 2/2] bust_spinlocks: do not decrement oops_in_progress unconditionally

On Tue, 13 Oct 2020, Paoloni, Gabriele wrote:

> In the current implementation if the input flag is 0
> oops_in_progress is unconditionally decremented, thus allowing
> to become a negative number. Since right now oops_in_progress
> is a global variable used in the kernel as a conditional flag
> to check if oops, panic(), BUG() or die() is in progress the
> current unconditional decrement may lead to unexpected behavior
> in the Kernel paths conditionally executing over this flag.
> 
> This patch only decrement oops_in_progress if it is non zero
> 
> Signed-off-by: Gabriele Paoloni <[email protected]>
> ---
>  lib/bust_spinlocks.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bust_spinlocks.c b/lib/bust_spinlocks.c
> index 594b270161d9..842633ac9130 100644
> --- a/lib/bust_spinlocks.c
> +++ b/lib/bust_spinlocks.c
> @@ -23,6 +23,9 @@
>   * @yes: input flag; if zero decreases oops_in_progress,
>   * otherwise increases it.
>   *
> + * Note: if oops_in_progress is already 0 it will not
> + * be decreased
> + *
>   */
>  void bust_spinlocks(int yes)
>  {
> @@ -33,7 +36,9 @@ void bust_spinlocks(int yes)
>  		unblank_screen();
>  #endif
>  		console_unblank();
> -		if (--oops_in_progress == 0)
> +		if (oops_in_progress)
> +			oops_in_progress--;
> +		if (!oops_in_progress)
>  			wake_up_klogd();

I did not get the original motivation stated above.

But I believe you meant:

'I think there is race condition here (before this patch).'

So do something in this patch: 'And now the race condition is gone?'


I think:

'The could be a race condition before, and probably the race condition is 
still there after this patch.'

But maybe I did even get the intent of this patch in the first place...

Lukas