]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
writeback: Add asserts for adding freed inode to lists
authorJan Kara <jack@suse.cz>
Mon, 12 Dec 2022 11:36:33 +0000 (12:36 +0100)
committerJens Axboe <axboe@kernel.dk>
Mon, 12 Dec 2022 20:08:34 +0000 (13:08 -0700)
In the past we had several use-after-free issues with inodes getting
added to writeback lists after evict() removed them. These are painful
to debug so add some asserts to catch the problem earlier. The only
non-obvious change in the commit is that we need to tweak
redirty_tail_locked() to avoid triggering assertion in
inode_io_list_move_locked().

Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/20221212113633.29181-1-jack@suse.cz
Signed-off-by: Jens Axboe <axboe@kernel.dk>
fs/fs-writeback.c

index 443f83382b9bdda308451ab285dccec25b4e9edc..6cd172c4cb3e2c278c6afbeb8e64dd338a6c4052 100644 (file)
@@ -121,6 +121,7 @@ static bool inode_io_list_move_locked(struct inode *inode,
 {
        assert_spin_locked(&wb->list_lock);
        assert_spin_locked(&inode->i_lock);
+       WARN_ON_ONCE(inode->i_state & I_FREEING);
 
        list_move(&inode->i_io_list, head);
 
@@ -280,6 +281,7 @@ static void inode_cgwb_move_to_attached(struct inode *inode,
 {
        assert_spin_locked(&wb->list_lock);
        assert_spin_locked(&inode->i_lock);
+       WARN_ON_ONCE(inode->i_state & I_FREEING);
 
        inode->i_state &= ~I_SYNC_QUEUED;
        if (wb != &wb->bdi->wb)
@@ -1129,6 +1131,7 @@ static void inode_cgwb_move_to_attached(struct inode *inode,
 {
        assert_spin_locked(&wb->list_lock);
        assert_spin_locked(&inode->i_lock);
+       WARN_ON_ONCE(inode->i_state & I_FREEING);
 
        inode->i_state &= ~I_SYNC_QUEUED;
        list_del_init(&inode->i_io_list);
@@ -1294,6 +1297,17 @@ static void redirty_tail_locked(struct inode *inode, struct bdi_writeback *wb)
 {
        assert_spin_locked(&inode->i_lock);
 
+       inode->i_state &= ~I_SYNC_QUEUED;
+       /*
+        * When the inode is being freed just don't bother with dirty list
+        * tracking. Flush worker will ignore this inode anyway and it will
+        * trigger assertions in inode_io_list_move_locked().
+        */
+       if (inode->i_state & I_FREEING) {
+               list_del_init(&inode->i_io_list);
+               wb_io_lists_depopulated(wb);
+               return;
+       }
        if (!list_empty(&wb->b_dirty)) {
                struct inode *tail;
 
@@ -1302,7 +1316,6 @@ static void redirty_tail_locked(struct inode *inode, struct bdi_writeback *wb)
                        inode->dirtied_when = jiffies;
        }
        inode_io_list_move_locked(inode, wb, &wb->b_dirty);
-       inode->i_state &= ~I_SYNC_QUEUED;
 }
 
 static void redirty_tail(struct inode *inode, struct bdi_writeback *wb)