[Devel] [PATCH RH7] vziolimit: fix race by putting throttle.speed check under ub_lock

[Konstantin Khorenko]

On 06/28/2019 02:47 PM, Pavel Tikhomirov wrote:
> We had a crash on these stack:
>
> iolimit_balance_dirty
>   if (!th->speed) # <- th->speed is non-zero here
>     return;
>   ...
>   throttle_charge(th, (long long)dirty << PAGE_SHIFT);
>     ...
>     if (do_div(step, th->speed)) # <- th->speed is zero here, crash
>
> We don't change th->speed in between, throttle.speed is changed from:
>
> 1) iolimit_cgroup_write_u64 (cgroup beancounter.iolimit.speed)
> 2) and iolimit_ioctl->throttle_setup
>
> Both are under spin_lock_irq(&ub->ub_lock); so putting the check under
> the same lock should fix the race.
>
> https://jira.sw.ru/browse/PSBM-95815
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

Acked-by: Konstantin Khorenko <khorenko at virtuozzo.com>

> ---
>  kernel/ve/vziolimit.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/ve/vziolimit.c b/kernel/ve/vziolimit.c
> index d1cd687ab1f1..d6bbc8c1dd89 100644
> --- a/kernel/ve/vziolimit.c
> +++ b/kernel/ve/vziolimit.c
> @@ -138,9 +138,6 @@ static void iolimit_balance_dirty(struct iolimit *iolimit,
>  	struct throttle *th = &iolimit->throttle;
>  	unsigned long flags, dirty, state;
>
> -	if (!th->speed)
> -		return;
> -
>  	/* can be non-atomic on i386, but ok. this just hint. */
>  	state = th->state >> PAGE_SHIFT;
>  	dirty = ub_stat_get(ub, dirty_pages) + write_chunk;
> @@ -154,7 +151,8 @@ static void iolimit_balance_dirty(struct iolimit *iolimit,
>
>  	spin_lock_irqsave(&ub->ub_lock, flags);
>  	/* precharge dirty pages */
> -	throttle_charge(th, (long long)dirty << PAGE_SHIFT);
> +	if (th->speed)
> +		throttle_charge(th, (long long)dirty << PAGE_SHIFT);
>  	spin_unlock_irqrestore(&ub->ub_lock, flags);
>  }
>
>