[PATCH RESEND] romfs: reject directory entry chains that do not move forward

[PATCH RESEND] romfs: reject directory entry chains that do not move forward

[Palla Raghunath]

romfs_lookup() and romfs_readdir() walk a directory's entry chain by
following ri.next, and stop only once the offset is zero or past the end
of the image. Nothing stops an entry from pointing at itself, so on a
crafted image a lookup of a name that is not present spins forever.

That loop has no cond_resched() and no signal check, so the task cannot
be killed, and it spins with i_rwsem held and the dentry left in the
in-lookup state. A second lookup of the same name then waits forever in
d_alloc_parallel(), and it is that waiter the hung task watchdog
reports, which makes this look like a dcache problem:

  INFO: task syz-executor239:5636 blocked for more than 143 seconds.
   d_alloc_parallel+0xd13/0x16c0 fs/dcache.c:2839
   __lookup_slow+0x82/0x2f0 fs/namei.c:1904

Require the chain to move forward and return -EIO if it does not. File
headers are laid out in increasing order and both loops already treat
the chain as a bounded forward-only walk, so this only enforces what
they assume. In romfs_readdir() a cycle never advances ctx->pos and has
getdents() repeat entries forever, so check there too.

The hard link chain bounded by commit d30b5a954e0a ("romfs: detect hard
link cycles") is a separate one. Sangho Lee posted a fix for this chain
in July 2026 that validates it at inode instantiation; it has had no
replies.

Reported-by: syzbot+e8ebfecec06a009dd2d3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e8ebfecec06a009dd2d3
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>
---
 fs/romfs/super.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 7f783d6173e8..846b57f265ef 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -143,7 +143,7 @@ static int romfs_readdir(struct file *file, struct dir_context *ctx)
 {
 	struct inode *i = file_inode(file);
 	struct romfs_inode ri;
-	unsigned long offset, maxoff;
+	unsigned long offset, maxoff, next;
 	int j, ino, nextfh;
 	char fsname[ROMFS_MAXFN];	/* XXX dynamic? */
 	int ret;
@@ -191,7 +191,16 @@ static int romfs_readdir(struct file *file, struct dir_context *ctx)
 			    romfs_dtype_table[nextfh & ROMFH_TYPE]))
 			goto out;
 
-		offset = nextfh & ROMFH_MASK;
+		/* likewise, a chain that does not move forward would have
+		 * getdents() repeat entries forever
+		 */
+		next = nextfh & ROMFH_MASK;
+		if (next && next <= offset) {
+			offset = maxoff;
+			ctx->pos = offset;
+			goto out;
+		}
+		offset = next;
 	}
 out:
 	return 0;
@@ -203,7 +212,7 @@ static int romfs_readdir(struct file *file, struct dir_context *ctx)
 static struct dentry *romfs_lookup(struct inode *dir, struct dentry *dentry,
 				   unsigned int flags)
 {
-	unsigned long offset, maxoff;
+	unsigned long offset, maxoff, next;
 	struct inode *inode = NULL;
 	struct romfs_inode ri;
 	const char *name;		/* got from dentry */
@@ -245,8 +254,15 @@ static struct dentry *romfs_lookup(struct inode *dir, struct dentry *dentry,
 			break;
 		}
 
-		/* next entry */
-		offset = be32_to_cpu(ri.next) & ROMFH_MASK;
+		/* next entry; the chain must move forward, or a corrupt image
+		 * will keep us here forever
+		 */
+		next = be32_to_cpu(ri.next) & ROMFH_MASK;
+		if (next && next <= offset) {
+			ret = -EIO;
+			goto error;
+		}
+		offset = next;
 	}
 
 	return d_splice_alias(inode, dentry);
-- 
2.34.1

Re: [PATCH RESEND] romfs: reject directory entry chains that do not move forward

[Jan Kara]

Adding Dave Howells since he put himself into the headers of this
filesystem :)

On Fri 18-09-26 17:46:41, Palla Raghunath wrote:
> romfs_lookup() and romfs_readdir() walk a directory's entry chain by
> following ri.next, and stop only once the offset is zero or past the end
> of the image. Nothing stops an entry from pointing at itself, so on a
> crafted image a lookup of a name that is not present spins forever.
> 
> That loop has no cond_resched() and no signal check, so the task cannot
> be killed, and it spins with i_rwsem held and the dentry left in the
> in-lookup state. A second lookup of the same name then waits forever in
> d_alloc_parallel(), and it is that waiter the hung task watchdog
> reports, which makes this look like a dcache problem:
> 
>   INFO: task syz-executor239:5636 blocked for more than 143 seconds.
>    d_alloc_parallel+0xd13/0x16c0 fs/dcache.c:2839
>    __lookup_slow+0x82/0x2f0 fs/namei.c:1904
> 
> Require the chain to move forward and return -EIO if it does not. File
> headers are laid out in increasing order and both loops already treat
> the chain as a bounded forward-only walk, so this only enforces what
> they assume. In romfs_readdir() a cycle never advances ctx->pos and has
> getdents() repeat entries forever, so check there too.
> 
> The hard link chain bounded by commit d30b5a954e0a ("romfs: detect hard
> link cycles") is a separate one. Sangho Lee posted a fix for this chain
> in July 2026 that validates it at inode instantiation; it has had no
> replies.
> 
> Reported-by: syzbot+e8ebfecec06a009dd2d3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e8ebfecec06a009dd2d3
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org

I doubt these bugs have any practical relevance so I wouldn't CC stable.
But I guess that's for Christian.

> Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>

The patch looks good, I'm just not sure we are guaranteed ROMFS directories
are indeed following monotonically increasing disk offset (although that's
the most logical thing to do) - added Dave to CC for that. Otherwise feel
free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/romfs/super.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/romfs/super.c b/fs/romfs/super.c
> index 7f783d6173e8..846b57f265ef 100644
> --- a/fs/romfs/super.c
> +++ b/fs/romfs/super.c
> @@ -143,7 +143,7 @@ static int romfs_readdir(struct file *file, struct dir_context *ctx)
>  {
>  	struct inode *i = file_inode(file);
>  	struct romfs_inode ri;
> -	unsigned long offset, maxoff;
> +	unsigned long offset, maxoff, next;
>  	int j, ino, nextfh;
>  	char fsname[ROMFS_MAXFN];	/* XXX dynamic? */
>  	int ret;
> @@ -191,7 +191,16 @@ static int romfs_readdir(struct file *file, struct dir_context *ctx)
>  			    romfs_dtype_table[nextfh & ROMFH_TYPE]))
>  			goto out;
>  
> -		offset = nextfh & ROMFH_MASK;
> +		/* likewise, a chain that does not move forward would have
> +		 * getdents() repeat entries forever
> +		 */
> +		next = nextfh & ROMFH_MASK;
> +		if (next && next <= offset) {
> +			offset = maxoff;
> +			ctx->pos = offset;
> +			goto out;
> +		}
> +		offset = next;
>  	}
>  out:
>  	return 0;
> @@ -203,7 +212,7 @@ static int romfs_readdir(struct file *file, struct dir_context *ctx)
>  static struct dentry *romfs_lookup(struct inode *dir, struct dentry *dentry,
>  				   unsigned int flags)
>  {
> -	unsigned long offset, maxoff;
> +	unsigned long offset, maxoff, next;
>  	struct inode *inode = NULL;
>  	struct romfs_inode ri;
>  	const char *name;		/* got from dentry */
> @@ -245,8 +254,15 @@ static struct dentry *romfs_lookup(struct inode *dir, struct dentry *dentry,
>  			break;
>  		}
>  
> -		/* next entry */
> -		offset = be32_to_cpu(ri.next) & ROMFH_MASK;
> +		/* next entry; the chain must move forward, or a corrupt image
> +		 * will keep us here forever
> +		 */
> +		next = be32_to_cpu(ri.next) & ROMFH_MASK;
> +		if (next && next <= offset) {
> +			ret = -EIO;
> +			goto error;
> +		}
> +		offset = next;
>  	}
>  
>  	return d_splice_alias(inode, dentry);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH RESEND] romfs: reject directory entry chains that do not move forward

[Palla Raghunath]

On Tue, Sep 22, 2026 at 11:58:35AM +0200, Jan Kara wrote:
> I doubt these bugs have any practical relevance so I wouldn't CC stable.
> But I guess that's for Christian.

Fair enough. I'll drop the stable tag if there is a v2.

> The patch looks good, I'm just not sure we are guaranteed ROMFS directories
> are indeed following monotonically increasing disk offset (although that's
> the most logical thing to do) - added Dave to CC for that.

Thanks for the review, Jan.

Documentation/filesystems/romfs.rst doesn't state it, but genromfs
always lays directories out that way. processdir() hands each entry the
current offset, which only ever grows, and it places a subdirectory's
contents straight after the subdirectory's own header, before the next
sibling. Entries are added to the directory list in that same order,
and dumpnode() writes each next pointer from that list, so within one
directory every next pointer points forward. I don't know of any other
tool that builds romfs images.

If you'd rather not rely on that, I can bound the walk by the number of
16-byte header slots in the image instead. That stops a cycle without
assuming any particular order. Happy to send a v2 either way.

Thanks,
Palla

Re: [PATCH RESEND] romfs: reject directory entry chains that do not move forward

[Jan Kara]

On Wed 30-09-26 18:33:28, Palla Raghunath wrote:
> On Tue, Sep 22, 2026 at 11:58:35AM +0200, Jan Kara wrote:
> > I doubt these bugs have any practical relevance so I wouldn't CC stable.
> > But I guess that's for Christian.
> 
> Fair enough. I'll drop the stable tag if there is a v2.
> 
> > The patch looks good, I'm just not sure we are guaranteed ROMFS directories
> > are indeed following monotonically increasing disk offset (although that's
> > the most logical thing to do) - added Dave to CC for that.
> 
> Thanks for the review, Jan.
> 
> Documentation/filesystems/romfs.rst doesn't state it, but genromfs
> always lays directories out that way. processdir() hands each entry the
> current offset, which only ever grows, and it places a subdirectory's
> contents straight after the subdirectory's own header, before the next
> sibling. Entries are added to the directory list in that same order,
> and dumpnode() writes each next pointer from that list, so within one
> directory every next pointer points forward. I don't know of any other
> tool that builds romfs images.
> 
> If you'd rather not rely on that, I can bound the walk by the number of
> 16-byte header slots in the image instead. That stops a cycle without
> assuming any particular order. Happy to send a v2 either way.

After checking Documentation/filesystems/romfs.rst I agree that monotonely
increasing offsets were likely the intention despite it isn't stated there
explicitely. So I think your current version is fine. Thanks!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR