linux-kernel - Re: [PATCH] Revert "libfs: Use d_children list to iterate simple

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a7fe0eda-78e4-43bb-822b-c1dfa65ba4dd@oracle.com>
Date: Wed, 26 Feb 2025 10:57:48 -0500
From: Chuck Lever <chuck.lever@...cle.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, viro@...iv.linux.org.uk,
        brauner@...nel.org, jack@...e.cz
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable <stable@...nel.org>, Takashi Iwai <tiwai@...e.de>
Subject: Re: [PATCH] Revert "libfs: Use d_children list to iterate
 simple_offset directories"

On 2/26/25 9:29 AM, Greg Kroah-Hartman wrote:
> This reverts commit b9b588f22a0c049a14885399e27625635ae6ef91.
> 
> There are reports of this commit breaking Chrome's rendering mode.  As
> no one seems to want to do a root-cause, let's just revert it for now as
> it is affecting people using the latest release as well as the stable
> kernels that it has been backported to.

NACK. This re-introduces a CVE.

The problem is we don't have a reproducer yet.


> Link: https://lore.kernel.org/r/874j0lvy89.wl-tiwai@suse.de
> Fixes: b9b588f22a0c ("libfs: Use d_children list to iterate simple_offset directories")
> Cc: stable <stable@...nel.org>
> Reported-by: Takashi Iwai <tiwai@...e.de>
> Cc: Chuck Lever <chuck.lever@...cle.com>
> Cc: Christian Brauner <brauner@...nel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>  fs/libfs.c | 90 ++++++++++++++++++------------------------------------
>  1 file changed, 29 insertions(+), 61 deletions(-)
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 8444f5cc4064..96f491f82f99 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -247,13 +247,12 @@ EXPORT_SYMBOL(simple_dir_inode_operations);
>  
>  /* simple_offset_add() never assigns these to a dentry */
>  enum {
> -	DIR_OFFSET_FIRST	= 2,		/* Find first real entry */
>  	DIR_OFFSET_EOD		= S32_MAX,
>  };
>  
>  /* simple_offset_add() allocation range */
>  enum {
> -	DIR_OFFSET_MIN		= DIR_OFFSET_FIRST + 1,
> +	DIR_OFFSET_MIN		= 2,
>  	DIR_OFFSET_MAX		= DIR_OFFSET_EOD - 1,
>  };
>  
> @@ -458,82 +457,51 @@ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence)
>  	return vfs_setpos(file, offset, LONG_MAX);
>  }
>  
> -static struct dentry *find_positive_dentry(struct dentry *parent,
> -					   struct dentry *dentry,
> -					   bool next)
> +static struct dentry *offset_find_next(struct offset_ctx *octx, loff_t offset)
>  {
> -	struct dentry *found = NULL;
> -
> -	spin_lock(&parent->d_lock);
> -	if (next)
> -		dentry = d_next_sibling(dentry);
> -	else if (!dentry)
> -		dentry = d_first_child(parent);
> -	hlist_for_each_entry_from(dentry, d_sib) {
> -		if (!simple_positive(dentry))
> -			continue;
> -		spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
> -		if (simple_positive(dentry))
> -			found = dget_dlock(dentry);
> -		spin_unlock(&dentry->d_lock);
> -		if (likely(found))
> -			break;
> -	}
> -	spin_unlock(&parent->d_lock);
> -	return found;
> -}
> -
> -static noinline_for_stack struct dentry *
> -offset_dir_lookup(struct dentry *parent, loff_t offset)
> -{
> -	struct inode *inode = d_inode(parent);
> -	struct offset_ctx *octx = inode->i_op->get_offset_ctx(inode);
> +	MA_STATE(mas, &octx->mt, offset, offset);
>  	struct dentry *child, *found = NULL;
>  
> -	MA_STATE(mas, &octx->mt, offset, offset);
> -
> -	if (offset == DIR_OFFSET_FIRST)
> -		found = find_positive_dentry(parent, NULL, false);
> -	else {
> -		rcu_read_lock();
> -		child = mas_find(&mas, DIR_OFFSET_MAX);
> -		found = find_positive_dentry(parent, child, false);
> -		rcu_read_unlock();
> -	}
> +	rcu_read_lock();
> +	child = mas_find(&mas, DIR_OFFSET_MAX);
> +	if (!child)
> +		goto out;
> +	spin_lock(&child->d_lock);
> +	if (simple_positive(child))
> +		found = dget_dlock(child);
> +	spin_unlock(&child->d_lock);
> +out:
> +	rcu_read_unlock();
>  	return found;
>  }
>  
>  static bool offset_dir_emit(struct dir_context *ctx, struct dentry *dentry)
>  {
>  	struct inode *inode = d_inode(dentry);
> +	long offset = dentry2offset(dentry);
>  
> -	return dir_emit(ctx, dentry->d_name.name, dentry->d_name.len,
> -			inode->i_ino, fs_umode_to_dtype(inode->i_mode));
> +	return ctx->actor(ctx, dentry->d_name.name, dentry->d_name.len, offset,
> +			  inode->i_ino, fs_umode_to_dtype(inode->i_mode));
>  }
>  
> -static void offset_iterate_dir(struct file *file, struct dir_context *ctx)
> +static void offset_iterate_dir(struct inode *inode, struct dir_context *ctx)
>  {
> -	struct dentry *dir = file->f_path.dentry;
> +	struct offset_ctx *octx = inode->i_op->get_offset_ctx(inode);
>  	struct dentry *dentry;
>  
> -	dentry = offset_dir_lookup(dir, ctx->pos);
> -	if (!dentry)
> -		goto out_eod;
>  	while (true) {
> -		struct dentry *next;
> -
> -		ctx->pos = dentry2offset(dentry);
> -		if (!offset_dir_emit(ctx, dentry))
> -			break;
> -
> -		next = find_positive_dentry(dir, dentry, true);
> -		dput(dentry);
> -
> -		if (!next)
> +		dentry = offset_find_next(octx, ctx->pos);
> +		if (!dentry)
>  			goto out_eod;
> -		dentry = next;
> +
> +		if (!offset_dir_emit(ctx, dentry)) {
> +			dput(dentry);
> +			break;
> +		}
> +
> +		ctx->pos = dentry2offset(dentry) + 1;
> +		dput(dentry);
>  	}
> -	dput(dentry);
>  	return;
>  
>  out_eod:
> @@ -572,7 +540,7 @@ static int offset_readdir(struct file *file, struct dir_context *ctx)
>  	if (!dir_emit_dots(file, ctx))
>  		return 0;
>  	if (ctx->pos != DIR_OFFSET_EOD)
> -		offset_iterate_dir(file, ctx);
> +		offset_iterate_dir(d_inode(dir), ctx);
>  	return 0;
>  }
>  


-- 
Chuck Lever