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

Pavel Tikhomirov ptikhomirov at virtuozzo.com
Fri Jun 28 14:47:01 MSK 2019


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>
---
 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);
 }
 
-- 
2.20.1



More information about the Devel mailing list