| 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
| ||
|
Message-ID: <c2kpawomkbvtahjm7y5mposbhckb7wxthi3iqy5yr22ggpucrm@ufvxwy233qxo>
Date: Mon, 24 Nov 2025 10:21:07 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: brauner@...nel.org, neil@...wn.name
Cc: agruenba@...hat.com, almaz.alexandrovich@...agon-software.com,
dhowells@...hat.com, gfs2@...ts.linux.dev, jack@...e.cz,
linux-afs@...ts.infradead.org, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
marc.dionne@...istor.com, ntfs3@...ts.linux.dev, syzkaller-bugs@...glegroups.com,
viro@...iv.linux.org.uk, syzbot+2fefb910d2c20c0698d8@...kaller.appspotmail.com
Subject: Re: [syzbot] [ntfs3?] INFO: task hung in __start_renaming
On Mon, Nov 24, 2025 at 10:01:53AM +0100, Mateusz Guzik wrote:
> sigh, so it *is* my patch, based on syzbot testing specifically on
> directory locking vs inode branches, but I don't see why.
>
> I take it the open() codepath took the rwsem, hence the rename is
> sleeping. Given that all reproducers find it *on* cpu, it may be this
> is busy looping for some reason.
>
> I don't have time to dig more into it right now, so I think it would
> be best to *drop* my patch for the time being. Once I figure it out
> I'll send a v2.
>
good news, now that I gave up I found it.
insert_inode_locked() is looping indefinitely an inode which is no
longer I_NEW or I_CREATING.
In stock kernel:
if (unlikely(!inode_unhashed(old))) {
iput(old);
return -EBUSY;
}
iput(old);
it returns an error
with my patch:
if (isnew) {
wait_on_new_inode(old);
if (unlikely(!inode_unhashed(old))) {
iput(old);
return -EBUSY;
}
}
iput(old);
unhashed status is only ever check if I_NEW was spotted,
which can be false. Afterwards the routine is stuck in endless cycle of
finding the inode and iputting it.
Christian, I think the easiest way out is to add the fix I initially
posted, inlined below. It *was* successfuly tested by syzbot. It retains
inode_unhashed checks even when they are not necessary to avoid any more
surprises.
There were some other changes in the area and turns out sending a v2 for
the patch would result in some merge conflicts, on the other hand the
patch below should be trivial to fold into the existing commit.
Sorry for the spam everyone. :-)
diff --git a/fs/inode.c b/fs/inode.c
index 0f3a56ea8f48..80298f048117 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1311,12 +1311,11 @@ struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
spin_unlock(&inode_hash_lock);
if (IS_ERR(old))
return NULL;
- if (unlikely(isnew)) {
+ if (unlikely(isnew))
wait_on_new_inode(old);
- if (unlikely(inode_unhashed(old))) {
- iput(old);
- goto again;
- }
+ if (unlikely(inode_unhashed(old))) {
+ iput(old);
+ goto again;
}
return old;
}
@@ -1413,12 +1412,11 @@ struct inode *iget5_locked_rcu(struct super_block *sb, unsigned long hashval,
if (inode) {
if (IS_ERR(inode))
return NULL;
- if (unlikely(isnew)) {
+ if (unlikely(isnew))
wait_on_new_inode(inode);
- if (unlikely(inode_unhashed(inode))) {
- iput(inode);
- goto again;
- }
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ goto again;
}
return inode;
}
@@ -1459,12 +1457,11 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino)
if (inode) {
if (IS_ERR(inode))
return NULL;
- if (unlikely(isnew)) {
+ if (unlikely(isnew))
wait_on_new_inode(inode);
- if (unlikely(inode_unhashed(inode))) {
- iput(inode);
- goto again;
- }
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ goto again;
}
return inode;
}
@@ -1501,12 +1498,11 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino)
if (IS_ERR(old))
return NULL;
inode = old;
- if (unlikely(isnew)) {
+ if (unlikely(isnew))
wait_on_new_inode(inode);
- if (unlikely(inode_unhashed(inode))) {
- iput(inode);
- goto again;
- }
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ goto again;
}
}
return inode;
@@ -1648,12 +1644,11 @@ struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
again:
inode = ilookup5_nowait(sb, hashval, test, data, &isnew);
if (inode) {
- if (unlikely(isnew)) {
+ if (unlikely(isnew))
wait_on_new_inode(inode);
- if (unlikely(inode_unhashed(inode))) {
- iput(inode);
- goto again;
- }
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ goto again;
}
}
return inode;
@@ -1682,12 +1677,11 @@ struct inode *ilookup(struct super_block *sb, unsigned long ino)
if (inode) {
if (IS_ERR(inode))
return NULL;
- if (unlikely(isnew)) {
+ if (unlikely(isnew))
wait_on_new_inode(inode);
- if (unlikely(inode_unhashed(inode))) {
- iput(inode);
- goto again;
- }
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ goto again;
}
}
return inode;
@@ -1863,12 +1857,11 @@ int insert_inode_locked(struct inode *inode)
isnew = !!(inode_state_read(old) & I_NEW);
spin_unlock(&old->i_lock);
spin_unlock(&inode_hash_lock);
- if (isnew) {
+ if (isnew)
wait_on_new_inode(old);
- if (unlikely(!inode_unhashed(old))) {
- iput(old);
- return -EBUSY;
- }
+ if (unlikely(!inode_unhashed(old))) {
+ iput(old);
+ return -EBUSY;
}
iput(old);
}
Powered by blists - more mailing lists