From patchwork Tue Apr 21 08:54:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 11500849 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 880161667 for ; Tue, 21 Apr 2020 08:54:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 706B321D79 for ; Tue, 21 Apr 2020 08:54:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728119AbgDUIyv (ORCPT ); Tue, 21 Apr 2020 04:54:51 -0400 Received: from mx2.suse.de ([195.135.220.15]:41504 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726628AbgDUIyu (ORCPT ); Tue, 21 Apr 2020 04:54:50 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C9B2DAC92; Tue, 21 Apr 2020 08:54:47 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 313F41E0E3B; Tue, 21 Apr 2020 10:54:48 +0200 (CEST) From: Jan Kara To: Ted Tso Cc: , , Eric Sandeen , Jan Kara Subject: [PATCH 1/3] fs: Avoid leaving freed inode on dirty list Date: Tue, 21 Apr 2020 10:54:43 +0200 Message-Id: <20200421085445.5731-2-jack@suse.cz> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200421085445.5731-1-jack@suse.cz> References: <20200421085445.5731-1-jack@suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org evict() can race with writeback_sb_inodes() and so list_empty(&inode->i_io_list) check can race with list_move() from redirty_tail() possibly resulting in list_empty() returning false and thus we end up leaving freed inode in wb->b_dirty list leading to use-after-free issues. Fix the problem by using list_empty_careful() check and add assert that inode's i_io_list is empty in clear_inode() to catch the problem earlier in the future. Signed-off-by: Jan Kara --- fs/inode.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/inode.c b/fs/inode.c index 93d9252a00ab..a73c8a7aa71a 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -534,6 +534,7 @@ void clear_inode(struct inode *inode) BUG_ON(!(inode->i_state & I_FREEING)); BUG_ON(inode->i_state & I_CLEAR); BUG_ON(!list_empty(&inode->i_wb_list)); + BUG_ON(!list_empty(&inode->i_io_list)); /* don't need i_lock here, no concurrent mods to i_state */ inode->i_state = I_FREEING | I_CLEAR; } @@ -559,7 +560,13 @@ static void evict(struct inode *inode) BUG_ON(!(inode->i_state & I_FREEING)); BUG_ON(!list_empty(&inode->i_lru)); - if (!list_empty(&inode->i_io_list)) + /* + * We are the only holder of the inode so it cannot be marked dirty. + * Flusher thread won't start new writeback but there can be still e.g. + * redirty_tail() running from writeback_sb_inodes(). So we have to be + * careful to remove inode from dirty/io list in all the cases. + */ + if (!list_empty_careful(&inode->i_io_list)) inode_io_list_del(inode); inode_sb_list_del(inode); From patchwork Tue Apr 21 08:54:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 11500845 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 77C651667 for ; Tue, 21 Apr 2020 08:54:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 681AD21D79 for ; Tue, 21 Apr 2020 08:54:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727891AbgDUIyu (ORCPT ); Tue, 21 Apr 2020 04:54:50 -0400 Received: from mx2.suse.de ([195.135.220.15]:41492 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726018AbgDUIyu (ORCPT ); Tue, 21 Apr 2020 04:54:50 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C9BE5ACAE; Tue, 21 Apr 2020 08:54:47 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 35A851E1233; Tue, 21 Apr 2020 10:54:48 +0200 (CEST) From: Jan Kara To: Ted Tso Cc: , , Eric Sandeen , Jan Kara Subject: [PATCH 2/3] writeback: Export inode_io_list_del() Date: Tue, 21 Apr 2020 10:54:44 +0200 Message-Id: <20200421085445.5731-3-jack@suse.cz> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200421085445.5731-1-jack@suse.cz> References: <20200421085445.5731-1-jack@suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Ext4 needs to remove inode from writeback lists after it is out of visibility of its journalling machinery (which can still dirty the inode). Export inode_io_list_del() for it. Signed-off-by: Jan Kara --- fs/fs-writeback.c | 1 + fs/internal.h | 2 -- include/linux/writeback.h | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 76ac9c7d32ec..e58bd5f758d0 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -1126,6 +1126,7 @@ void inode_io_list_del(struct inode *inode) inode_io_list_del_locked(inode, wb); spin_unlock(&wb->list_lock); } +EXPORT_SYMBOL(inode_io_list_del); /* * mark an inode as under writeback on the sb diff --git a/fs/internal.h b/fs/internal.h index aa5d45524e87..8819d0d58b03 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -143,8 +143,6 @@ extern int dentry_needs_remove_privs(struct dentry *dentry); /* * fs-writeback.c */ -extern void inode_io_list_del(struct inode *inode); - extern long get_nr_dirty_inodes(void); extern int invalidate_inodes(struct super_block *, bool); diff --git a/include/linux/writeback.h b/include/linux/writeback.h index a19d845dd7eb..902aa317621b 100644 --- a/include/linux/writeback.h +++ b/include/linux/writeback.h @@ -197,6 +197,7 @@ void wakeup_flusher_threads(enum wb_reason reason); void wakeup_flusher_threads_bdi(struct backing_dev_info *bdi, enum wb_reason reason); void inode_wait_for_writeback(struct inode *inode); +void inode_io_list_del(struct inode *inode); /* writeback.h requires fs.h; it, too, is not included from here. */ static inline void wait_on_inode(struct inode *inode) From patchwork Tue Apr 21 08:54:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 11500847 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 19C0781 for ; Tue, 21 Apr 2020 08:54:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0260920CC7 for ; Tue, 21 Apr 2020 08:54:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728003AbgDUIyv (ORCPT ); Tue, 21 Apr 2020 04:54:51 -0400 Received: from mx2.suse.de ([195.135.220.15]:41498 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726052AbgDUIyu (ORCPT ); Tue, 21 Apr 2020 04:54:50 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id CC380ACD0; Tue, 21 Apr 2020 08:54:47 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 395121E124D; Tue, 21 Apr 2020 10:54:48 +0200 (CEST) From: Jan Kara To: Ted Tso Cc: , , Eric Sandeen , Jan Kara Subject: [PATCH 3/3] ext4: Avoid freeing inodes on dirty list Date: Tue, 21 Apr 2020 10:54:45 +0200 Message-Id: <20200421085445.5731-4-jack@suse.cz> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200421085445.5731-1-jack@suse.cz> References: <20200421085445.5731-1-jack@suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org When we are evicting inode with journalled data, we may race with transaction commit in the following way: CPU0 CPU1 jbd2_journal_commit_transaction() evict(inode) inode_io_list_del() inode_wait_for_writeback() process BJ_Forget list __jbd2_journal_insert_checkpoint() __jbd2_journal_refile_buffer() __jbd2_journal_unfile_buffer() if (test_clear_buffer_jbddirty(bh)) mark_buffer_dirty(bh) __mark_inode_dirty(inode) ext4_evict_inode(inode) frees the inode This results in use-after-free issues in the writeback code (or the assertion added in the previous commit triggering). Fix the problem by removing inode from writeback lists once all the page cache is evicted and so inode cannot be added to writeback lists again. Signed-off-by: Jan Kara --- fs/ext4/inode.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index e416096fc081..d8a9d3da678c 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -220,6 +220,16 @@ void ext4_evict_inode(struct inode *inode) ext4_begin_ordered_truncate(inode, 0); truncate_inode_pages_final(&inode->i_data); + /* + * For inodes with journalled data, transaction commit could have + * dirtied the inode. Flush worker is ignoring it because of I_FREEING + * flag but we still need to remove the inode from the writeback lists. + */ + if (!list_empty_careful(&inode->i_io_list)) { + WARN_ON_ONCE(!ext4_should_journal_data(inode)); + inode_io_list_del(inode); + } + /* * Protect us against freezing - iput() caller didn't have to have any * protection against it