linux-ext4 - Re: 4.1-rc2 kvm-xfstests regressions for ext3 test config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.1505121059060.20717@localhost.localdomain>
Date:	Tue, 12 May 2015 11:20:23 +0200 (CEST)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	Eric Whitney <enwlinux@...il.com>
cc:	linux-ext4@...r.kernel.org, tytso@....edu, dccitaliano@...il.com
Subject: Re: 4.1-rc2 kvm-xfstests regressions for ext3 test config

On Mon, 11 May 2015, Eric Whitney wrote:

> Date: Mon, 11 May 2015 17:37:31 -0400
> From: Eric Whitney <enwlinux@...il.com>
> To: linux-ext4@...r.kernel.org
> Cc: tytso@....edu, dccitaliano@...il.com
> Subject: 4.1-rc2 kvm-xfstests regressions for ext3 test config
> 
> I've got six new test failures in the ext3 test configuration when
> running a 4.1-rc2 kernel on x86_64 with the latest version of
> kvm-xfstests.
> 
> The new failures include generic/075, generic/091, generic/112,
> generic/127, generic/231, and generic/263.  All also fail on ARM
> (Pandaboard) when running a 4.1-rc2 kernel on bare metal.
> 
> Reverting an -rc2 patch corrects all these failures:
> ext4: move check under lock scope to close a race (280227a75b56)
> 
> All the failed tests use fsx, all fail when attempting to perform a
> COLLAPSE_RANGE, and fsx uses this statement to determine whether a
> particular fallocate mode is supported (and then doesn't use it if not):
> 
> 	if (fallocate(fd, mode, 0, 1) && errno == EOPNOTSUPP)
> 
> In -rc1, ext4_fallocate()/extents.c returns EOPNOTSUPP, and in -rc2,
> EINVAL.  What's happened is that we no longer immediately fail a
> COLLAPSE_RANGE request for a block-mapped file with EOPNOTSUPP.  Instead,
> we now first check to see if the COLLAPSE_RANGE request is cluster-aligned
> and sized in ext4_collapse_range()/extents.c, and return EINVAL if not.
> (There's still a check for block-mapped files, but it's applied later in
> ext4_collapse_range().)
> 
> I think we'd want to return EOPNOTSUPP first in this case.
> 
> Eric

Thanks for the heads up. Yes, this is a problem and it has been
ported to stable as well. I think that the best solution would
be to have the check at both places to avoid unnecessary work
in most cases.

Something like this untested patch


diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d74e0802..fd2939f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4927,6 +4927,11 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	if (ret)
 		return ret;
 
+	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
 	if (mode & FALLOC_FL_COLLAPSE_RANGE)
 		return ext4_collapse_range(inode, offset, len);
 
@@ -5431,6 +5436,12 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	/* Take mutex lock */
 	mutex_lock(&inode->i_mutex);
 
+	/* Currently just for extent based files */
+	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
+		ret = -EOPNOTSUPP;
+		goto out_mutex;
+	}
+
 	/*
 	 * There is no need to overlap collapse range with EOF, in which case
 	 * it is effectively a truncate operation
@@ -5440,12 +5451,6 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 		goto out_mutex;
 	}
 
-	/* Currently just for extent based files */
-	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
-		ret = -EOPNOTSUPP;
-		goto out_mutex;
-	}
-
 	truncate_pagecache(inode, ioffset);
 
 	/* Wait for existing dio to complete */

> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html