From patchwork Thu Jul 9 17:45:45 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Czerner X-Patchwork-Id: 6757711 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 19B179F319 for ; Thu, 9 Jul 2015 17:46:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 328CC20607 for ; Thu, 9 Jul 2015 17:46:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1B4E320606 for ; Thu, 9 Jul 2015 17:46:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752210AbbGIRqA (ORCPT ); Thu, 9 Jul 2015 13:46:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54376 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751164AbbGIRqA (ORCPT ); Thu, 9 Jul 2015 13:46:00 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 9CD352B51C1; Thu, 9 Jul 2015 17:45:59 +0000 (UTC) Received: from localhost.localdomain.com (vpn1-4-112.ams2.redhat.com [10.36.4.112]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t69Hjt1i024008; Thu, 9 Jul 2015 13:45:56 -0400 From: Lukas Czerner To: viro@ZenIV.linux.org.uk Cc: bfields@fieldses.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Lukas Czerner Subject: [RFC][PATCH] fs: Prevent syncing frozen file system Date: Thu, 9 Jul 2015 19:45:45 +0200 Message-Id: <1436463945-12556-1-git-send-email-lczerner@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently we can end up in a deadlock because of broken sb_start_write -> s_umount ordering. The race goes like this: - write the file - unlink the file - final_iput will not be calles as file is opened - freeze the file system - Now simultaneously close the file and call sync (or syncfs on that particular file system). Sync will get to wait_sb_inodes() where it will grab the referece to the inode (__iget()) and later to call iput(). If we manage to close the file and drop the reference in between those calls sync will attempt to do a iput_final() because the inode is now unlinked and we're holding the last reference to it. This will however block on a frozen file system (ext4_delete_inode for example). Note that I've not been able to reproduce the issue, I've only seen this happen once. However with some instrumentation (like msleep() in the wait_sb_inodes() it can be achieved. Fix this by properly doing sb_start_write/sb_end_write to prevent us from fsfreeze. Note that with this patch syncfs will block on the frozen file system which is probably ok, but sync will block if any file system happens to be frozen - not sure if that's a problem, but it's certainly different from what we've been used to. Signed-off-by: Lukas Czerner --- fs/super.c | 7 +++++++ fs/sync.c | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/fs/super.c b/fs/super.c index b613723..d337c91 100644 --- a/fs/super.c +++ b/fs/super.c @@ -514,10 +514,17 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg) sb->s_count++; spin_unlock(&sb_lock); + /* + * Whatever we're going to do to the file system we have to + * make sure that we'll not end up blocking on frozen file + * system. + */ + sb_start_write(sb); down_read(&sb->s_umount); if (sb->s_root && (sb->s_flags & MS_BORN)) f(sb, arg); up_read(&sb->s_umount); + sb_end_write(sb); spin_lock(&sb_lock); if (p) diff --git a/fs/sync.c b/fs/sync.c index fbc98ee..074247f 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -156,9 +156,17 @@ SYSCALL_DEFINE1(syncfs, int, fd) return -EBADF; sb = f.file->f_path.dentry->d_sb; + /* + * If the file system is frozen we can't proceed because we + * could potentially block on frozen file system. This would + * lead to a deadlock, because we'll be holding s_umount which + * has to be taken in order to thaw the file system as well + */ + sb_start_write(sb); down_read(&sb->s_umount); ret = sync_filesystem(sb); up_read(&sb->s_umount); + sb_end_write(sb); fdput(f); return ret;