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-next>] [day] [month] [year] [list]
Date:	Mon, 14 Dec 2015 18:59:15 +0100
From:	Vegard Nossum <vegard.nossum@...cle.com>
To:	tytso@....edu, adilger.kernel@...ger.ca
Cc:	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
	Vegard Nossum <vegard.nossum@...cle.com>
Subject: [PATCH] ext4: handle errors during orphan cleanup

If a filesystem is mounted with errors=remount-ro, then orphan cleanup
can enter an infinite loop since the iput() inside the linked list
traversal doesn't actually always cause es->s_last_orphan to advance to
the next orphan inode (i.e. in case of errors).

The bug manifests in two different ways. It's an endless spew of either:

  EXT4-fs (loop0): Inode 5 (ffff8800153ed720): orphan list check failed!
  [...]
  CPU: 1 PID: 957 Comm: mount Not tainted 4.4.0-rc3+ #244
   ffffffff820ac0c0 ffff88001562f868 ffffffff81610cc9 ffff8800153ed7e0
   ffff88001562f8a0 ffffffff8133097a 00000000000003e8 ffffffff00000001
   ffff8800153ed7e0 ffffffff820ac0c0 ffff8800153ed880 ffff88001562f8c0
  Call Trace:
   [<ffffffff81610cc9>] dump_stack+0x44/0x5b
   [<ffffffff8133097a>] ext4_destroy_inode+0xba/0xc0
   [<ffffffff8125440f>] destroy_inode+0x5f/0x80
   [<ffffffff81254d75>] evict+0x1e5/0x270
   [<ffffffff81256217>] iput+0x297/0x350
   [<ffffffff813393c5>] ext4_fill_super+0x4fa5/0x53b0
  [...]

or:

  WARNING: CPU: 0 PID: 924 at lib/list_debug.c:36 __list_add+0xf9/0x100()
  list_add double add: new=00000000dfba0070, prev=00000000dffba970, next=00000000dfba0070.
  CPU: 0 PID: 924 Comm: mount.exe Tainted: G        W       4.4.0-rc3 #1
  Stack:
   df7f59b0 60075642 6071c3ae 00000009
   df7f5a30 600bc4fe df7f59c0 603f1e5f
   df7f5a20 600412cd df7f59e0 6040d859
  Call Trace:
   [<60029f9b>] show_stack+0xdb/0x1a0
   [<603f1e5f>] dump_stack+0x2a/0x3b
   [<600412cd>] warn_slowpath_common+0x9d/0xf0
   [<600413f4>] warn_slowpath_fmt+0x94/0xa0
   [<6040d859>] __list_add+0xf9/0x100
   [<601b28d4>] ext4_fill_super+0x3e04/0x4040
  [...]

This was the smallest change I could find that still covers all the
cases I ran into. It probably also makes sense intuitively to not
continue orphan cleanup if there was an error in the meantime.

Thanks to Jamie and Quentin for helping with the debugging.

Signed-off-by: Vegard Nossum <vegard.nossum@...cle.com>
Cc: Jamie Iles <jamie.iles@...cle.com>
Cc: Quentin Casasnovas <quentin.casasnovas@...cle.com>
Cc: stable@...r.kernel.org
---
 fs/ext4/super.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git fs/ext4/super.c fs/ext4/super.c
index c9ab67d..8952fcc 100644
--- fs/ext4/super.c
+++ fs/ext4/super.c
@@ -2206,17 +2206,6 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 		return;
 	}
 
-	if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
-		/* don't clear list on RO mount w/ errors */
-		if (es->s_last_orphan && !(s_flags & MS_RDONLY)) {
-			ext4_msg(sb, KERN_INFO, "Errors on filesystem, "
-				  "clearing orphan list.\n");
-			es->s_last_orphan = 0;
-		}
-		jbd_debug(1, "Skipping orphan recovery on fs with errors.\n");
-		return;
-	}
-
 	if (s_flags & MS_RDONLY) {
 		ext4_msg(sb, KERN_INFO, "orphan cleanup on readonly fs");
 		sb->s_flags &= ~MS_RDONLY;
@@ -2239,6 +2228,17 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 	while (es->s_last_orphan) {
 		struct inode *inode;
 
+		if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
+			/* don't clear list on RO mount w/ errors */
+			if (es->s_last_orphan && !(s_flags & MS_RDONLY)) {
+				ext4_msg(sb, KERN_INFO, "Errors on filesystem, "
+					  "clearing orphan list.\n");
+				es->s_last_orphan = 0;
+			}
+			jbd_debug(1, "Skipping rest of orphan recovery on fs with errors.\n");
+			break;
+		}
+
 		inode = ext4_orphan_get(sb, le32_to_cpu(es->s_last_orphan));
 		if (IS_ERR(inode)) {
 			es->s_last_orphan = 0;
-- 
1.9.1

--
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

Powered by blists - more mailing lists