[PATCH] cgroup: arm the notify timer only while the file exists

[PATCH] cgroup: arm the notify timer only while the file exists

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@columbia.edu>

cgroup_rm_file() clears cfile->kn under cfile->lock and then deletes the
notify timer.  Since commit 4616120fca7f ("cgroup: add lockless
fast-path checks to cgroup_file_notify()"), cgroup_file_notify() checks
kn and arms the timer without that lock and can arm it right after it
was deleted:

  Task A (memcg charge)                  Task B (subtree_control write)
  cgroup_file_notify()
    READ_ONCE(cfile->kn) != NULL
                                         cgroup_rm_file()
                                           cfile->kn = NULL
                                           timer_delete_sync()
    timer_reduce(&cfile->notify_timer)
      /* timer queued again */

If the css is only hidden, showing it again runs timer_setup() on the
still queued timer and corrupts the timer wheel:

  [   14.324093] ODEBUG: init active (active state 0) object: 0000000009e526fd object type: timer_list hint: cgroup_file_notify_timer+0x0/0x10
  [   14.324123] WARNING: lib/debugobjects.c:629 at debug_print_object+0x1a4/0x3b0, CPU#1: cgnotify/64
  [   14.329806] CPU: 1 UID: 0 PID: 64 Comm: cgnotify Tainted: G        W            7.3.0-17087-gd865f2677c44 #1 PREEMPT
  [   14.335109] RIP: 0010:debug_print_object+0x1ae/0x3b0
  [   14.354351] Call Trace:
  [   14.354927]  <TASK>
  [   14.356650]  __debug_object_init+0x203/0x3c0
  [   14.361799]  timer_init_key+0x2c/0x240
  [   14.362682]  cgroup_addrm_files+0x754/0xc40
  [   14.366744]  css_populate_dir+0x12e/0x440
  [   14.368636]  cgroup_apply_control_enable+0x2f3/0xb60
  [   14.371769]  cgroup_subtree_control_write+0x740/0xc10
  [   14.376924]  cgroup_file_write+0x19e/0x700
  [   14.381809]  kernfs_fop_write_iter+0x337/0x510
  [   14.382818]  vfs_write+0x545/0x1250
  [   14.386491]  ksys_write+0xfa/0x200
  [   14.390144]  do_syscall_64+0xfb/0x590
  [   14.390994]  entry_SYSCALL_64_after_hwframe+0x77/0x7f

If the css is freed instead, the timer can fire on freed memory.

Arm the timer under cfile->lock after checking kn again and keep only
the kn and timer_pending() checks outside the lock.  With the fix, the
timer stays idle once the file is removed.

This bug was identified and confirmed with the help of a deterministic
hypervisor currently under development.  The C reproducer below however
hits it within a minute.

Fixes: 4616120fca7f ("cgroup: add lockless fast-path checks to cgroup_file_notify()")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/cgnotify.c
---
 kernel/cgroup/cgroup.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index a284225f8dc9..b3657e72db99 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4806,19 +4806,21 @@ void cgroup_file_notify(struct cgroup_file *cfile)
 	if (!READ_ONCE(cfile->kn))
 		return;
 
-	last = READ_ONCE(cfile->notified_at);
-	next = last + CGROUP_FILE_NOTIFY_MIN_INTV;
-	if (time_in_range(jiffies, last, next)) {
-		timer_reduce(&cfile->notify_timer, next);
-		if (timer_pending(&cfile->notify_timer))
-			return;
-	}
+	if (timer_pending(&cfile->notify_timer))
+		return;
 
 	spin_lock_irqsave(&cfile->lock, flags);
 	if (cfile->kn) {
-		kn = cfile->kn;
-		kernfs_get(kn);
-		WRITE_ONCE(cfile->notified_at, jiffies);
+		last = cfile->notified_at;
+		next = last + CGROUP_FILE_NOTIFY_MIN_INTV;
+
+		if (time_in_range(jiffies, last, next)) {
+			timer_reduce(&cfile->notify_timer, next);
+		} else {
+			kn = cfile->kn;
+			kernfs_get(kn);
+			cfile->notified_at = jiffies;
+		}
 	}
 	spin_unlock_irqrestore(&cfile->lock, flags);
 

---
base-commit: cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a
change-id: 20261101-cgroup-notify-timer-0177935cef2d

Best regards,
--  
Mani Tofigh <mt4022@columbia.edu>

Re: [PATCH] cgroup: arm the notify timer only while the file exists

[Tejun Heo]

Applied to cgroup/for-7.4-fixes.

Thanks.

--
tejun