[RHEL7,COMMIT] vziolimit: fix race by putting throttle.speed check under ub_lock

Submitted by Konstantin Khorenko on July 1, 2019, 8:26 a.m.

Details

Message ID 201907010826.x618QYGG032157@finist-ce7.sw.ru
State New
Series "vziolimit: fix race by putting throttle.speed check under ub_lock"
Headers show

Commit Message

Konstantin Khorenko July 1, 2019, 8:26 a.m.
The commit is pushed to "branch-rh7-3.10.0-957.21.3.vz7.106.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-957.21.3.vz7.106.3
------>
commit a7614ef828481c26b2cff3af4bb2d6467cacb919
Author: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Date:   Mon Jul 1 11:26:33 2019 +0300

    vziolimit: fix race by putting throttle.speed check under ub_lock
    
    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@virtuozzo.com>
    Acked-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 kernel/ve/vziolimit.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

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