linux-ext4 - Re: [bug report] ext4: Fix possible corruption when moving a directory

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230318020743.GO860405@mit.edu>
Date:   Fri, 17 Mar 2023 22:07:43 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Jan Kara <jack@...e.cz>
Cc:     Eric Biggers <ebiggers@...nel.org>,
        Dan Carpenter <error27@...il.com>, linux-ext4@...r.kernel.org
Subject: Re: [bug report] ext4: Fix possible corruption when moving a
 directory

On Wed, Mar 08, 2023 at 11:42:34AM +0100, Jan Kara wrote:
> > That analysis looks correct.  FYI, I think this is the same as the syzbot report
> > "[ext4?] WARNING: bad unlock balance in ext4_rename2"
> > (https://lore.kernel.org/linux-ext4/000000000000435c6905f639ae8e@google.com).
> 
> Good spotting! This should be fixed (along with the lock ordering problem)
> by 3c92792da8506 ("ext4: Fix deadlock during directory rename") Ted has
> just merged couple hours ago.

Unfortunately, the Syzkaller report is still triggering after the
merge and commit 3c92792da8506.  The double unlock is still there, and
so the following fix is still needed (which I will be sending to Linus).

       		     	      	     	    - Ted

>From 70e42feab2e20618ddd0cbfc4ab4b08628236ecd Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@....edu>
Date: Fri, 17 Mar 2023 21:53:52 -0400
Subject: [PATCH] ext4: fix possible double unlock when moving a directory

Fixes: 0813299c586b ("ext4: Fix possible corruption when moving a directory")
Link: https://lore.kernel.org/r/5efbe1b9-ad8b-4a4f-b422-24824d2b775c@kili.mountain
Reported-by: Dan Carpenter <error27@...il.com>
Reported-by: syzbot+0c73d1d8b952c5f3d714@...kaller.appspotmail.com
Signed-off-by: Theodore Ts'o <tytso@....edu>
---
 fs/ext4/namei.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 31e21de56432..a5010b5b8a8c 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3884,10 +3884,8 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 				goto end_rename;
 		}
 		retval = ext4_rename_dir_prepare(handle, &old);
-		if (retval) {
-			inode_unlock(old.inode);
+		if (retval)
 			goto end_rename;
-		}
 	}
 	/*
 	 * If we're renaming a file within an inline_data dir and adding or
-- 
2.31.0