On Fri, Jul 26, 2024 at 11:02:35AM GMT, Paul E. McKenney wrote: > On Fri, Jul 26, 2024 at 03:54:28PM +0200, Christian Brauner wrote: > > Hey, > > > > I could use some help with understanding a bug related to rcu that was > > reported today. It first seems to have shown up on the 25th of July: > > > > https://syzkaller.appspot.com/bug?extid=20d7e439f76bbbd863a7 > > > > We seem to be hitting the WARN_ON_ONCE() in: > > > > void rcu_sync_dtor(struct rcu_sync *rsp) > > { > > int gp_state; > > > > WARN_ON_ONCE(READ_ONCE(rsp->gp_state) == GP_PASSED); > > > > from destroy_super_work() which gets called when a superblock is really freed. > > > > If the superblock has been visible in userspace we do it via call_rcu(): > > > > static void destroy_super_work(struct work_struct *work) > > { > > struct super_block *s = container_of(work, struct super_block, > > destroy_work); > > fsnotify_sb_free(s); > > security_sb_free(s); > > put_user_ns(s->s_user_ns); > > kfree(s->s_subtype); > > for (int i = 0; i < SB_FREEZE_LEVELS; i++) > > percpu_free_rwsem(&s->s_writers.rw_sem[i]); > > kfree(s); > > } > > > > static void destroy_super_rcu(struct rcu_head *head) > > { > > struct super_block *s = container_of(head, struct super_block, rcu); > > INIT_WORK(&s->destroy_work, destroy_super_work); > > schedule_work(&s->destroy_work); > > } > > > > And I'm really confused because I don't understand the details for sync > > rcu enough to come up with a clear problem statement even. Could someone > > please explain what the WARN_ON_ONCE() is about? > > If I am not too confused (and Oleg will correct me if I am), this is > checking a use-after-free error. A given rcu_sync structure normally > transitions from GP_IDLE->GP_ENTER->GP_PASSED->GP_EXIT->GP_IDLE, with > possible side-trips from the GP_EXIT state through GP_REPLAY and back > to GP_EXIT in special cases such as during early boot. > > One way that this can happen is calling percpu_free_rwsem() without > having previously done the percpu_up_write() needed to match an earlier > percpu_down_write(). I don't see any other way that this can happen > right off-hand, but someone might have added something, or I might be > suffering a failure of imagination. And you were spot on of course! > > > (I had been wondering if this is related to commit 7180f8d91fcb ("vfs: > > add rcu-based find_inode variants for iget ops") and the fix in commit > > 5bc9ad78c2f8 ("vfs: handle __wait_on_freeing_inode() and evict() race") > > but I don't see how.) > > I don't see how either, unless it introduced some timing change > that made the scenario above more probable. > > > Thanks for taking a look! > > Hope this helps! If not, you know where to find me. ;-) Thank you for taking the time, Paul. That was indeed helpful!
