[PATCH] jfs: Check for discrepancies between iag and inomap iagctl

[PATCH] jfs: Check for discrepancies between iag and inomap iagctl

[Nikola Z. Ivanov]

syzbot reports "VFS: Busy inodes after unmount" which is caused
by a deadlock in the jfsCommit thread, in a call to diFree.

The filesystem from the syz repro can be used to reproduce the
deadlock by mounting it, deleting a file and running "sync" or
unmounting the filesystem. Running "fsck" on the filesystem
image seemingly fixes the inconsistency and we no longer deadlock.

This happens because the IAG that contains the inode we are
freeing is head of the free list, but also holds nfreeinos = 0,
which will lead to a deadlock when we try to add it as head
of the free list, since it already is head of the list.

Fix this by adding a check for the discrepancy before
reading more metapages that could potentially match with "mp".
Do the same for the extent free list as it may lead to a
similar deadlock.

Reported-by: syzbot+d569e274f46ca86f78fa@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d569e274f46ca86f78fa
Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
---
 fs/jfs/jfs_imap.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index ecb8e05b8b84..fd9f9babaf46 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -902,6 +902,25 @@ int diFree(struct inode *ip)
 	}
 	iagp = (struct iag *) mp->data;
 
+	/* We will deadlock if due to inconsistency
+	 * the iag has no free inodes/extents but is
+	 * head of the respective free list
+	 */
+	if (iagp->nfreeinos == 0 && imap->im_agctl[agno].inofree == iagno) {
+		IREAD_UNLOCK(ipimap);
+		AG_UNLOCK(imap, agno);
+		release_metapage(mp);
+		jfs_error(ip->i_sb, "nfreeinos = 0, but iag is head of freelist\n");
+		return -EIO;
+	}
+	if (iagp->nfreeexts == 0 && imap->im_agctl[agno].extfree == iagno) {
+		IREAD_UNLOCK(ipimap);
+		AG_UNLOCK(imap, agno);
+		release_metapage(mp);
+		jfs_error(ip->i_sb, "nfreeexts = 0, but iag is head of freelist\n");
+		return -EIO;
+	}
+
 	/* get the inode number and extent number of the inode within
 	 * the iag and the inode number within the extent.
 	 */
-- 
2.51.0

Re: [PATCH] jfs: Check for discrepancies between iag and inomap iagctl

[Nikola Z. Ivanov]

On 1/2/26 8:48 PM, Nikola Z. Ivanov wrote:
> syzbot reports "VFS: Busy inodes after unmount" which is caused
> by a deadlock in the jfsCommit thread, in a call to diFree.
>
> The filesystem from the syz repro can be used to reproduce the
> deadlock by mounting it, deleting a file and running "sync" or
> unmounting the filesystem. Running "fsck" on the filesystem
> image seemingly fixes the inconsistency and we no longer deadlock.
>
> This happens because the IAG that contains the inode we are
> freeing is head of the free list, but also holds nfreeinos = 0,
> which will lead to a deadlock when we try to add it as head
> of the free list, since it already is head of the list.
>
> Fix this by adding a check for the discrepancy before
> reading more metapages that could potentially match with "mp".
> Do the same for the extent free list as it may lead to a
> similar deadlock.
>
> Reported-by: syzbot+d569e274f46ca86f78fa@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d569e274f46ca86f78fa
> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
> ---
>   fs/jfs/jfs_imap.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index ecb8e05b8b84..fd9f9babaf46 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -902,6 +902,25 @@ int diFree(struct inode *ip)
>   	}
>   	iagp = (struct iag *) mp->data;
>   
> +	/* We will deadlock if due to inconsistency
> +	 * the iag has no free inodes/extents but is
> +	 * head of the respective free list
> +	 */
> +	if (iagp->nfreeinos == 0 && imap->im_agctl[agno].inofree == iagno) {
> +		IREAD_UNLOCK(ipimap);
> +		AG_UNLOCK(imap, agno);
> +		release_metapage(mp);
> +		jfs_error(ip->i_sb, "nfreeinos = 0, but iag is head of freelist\n");
> +		return -EIO;
> +	}
> +	if (iagp->nfreeexts == 0 && imap->im_agctl[agno].extfree == iagno) {
> +		IREAD_UNLOCK(ipimap);
> +		AG_UNLOCK(imap, agno);
> +		release_metapage(mp);
> +		jfs_error(ip->i_sb, "nfreeexts = 0, but iag is head of freelist\n");
> +		return -EIO;
> +	}
> +
>   	/* get the inode number and extent number of the inode within
>   	 * the iag and the inode number within the extent.
>   	 */

Hello,


Kindly sending a reminder on this in case this slipped through the cracks.

In case jfs is at the point where syzkaller bugs are not worth the

effort please let us know since I was unable to find a definitive

answer in the mail archive.


Thank you!