Re: [PATCH] libfs: fix infinite directory reads for offset dir

On 1/28/25 10:03 AM, ciprietti@google.com wrote:

From: yangerkun yangerkun@huawei.com

[ Upstream commit 64a7ce76fb901bf9f9c36cf5d681328fc0fd4b5a ]

After we switch tmpfs dir operations from simple_dir_operations to simple_offset_dir_operations, every rename happened will fill new dentry to dest dir's maple tree(&SHMEM_I(inode)->dir_offsets->mt) with a free key starting with octx->newx_offset, and then set newx_offset equals to free key + 1. This will lead to infinite readdir combine with rename happened at the same time, which fail generic/736 in xfstests(detail show as below).

1. create 5000 files(1 2 3...) under one dir
2. call readdir(man 3 readdir) once, and get one entry
3. rename(entry, "TEMPFILE"), then rename("TEMPFILE", entry)
4. loop 2~3, until readdir return nothing or we loop too many times(tmpfs break test with the second condition)

We choose the same logic what commit 9b378f6ad48cf ("btrfs: fix infinite directory reads") to fix it, record the last_index when we open dir, and do not emit the entry which index >= last_index. The file->private_data now used in offset dir can use directly to do this, and we also update the last_index when we llseek the dir file.

Fixes: a2e459555c5f ("shmem: stable directory offsets") Signed-off-by: yangerkun yangerkun@huawei.com Link: https://lore.kernel.org/r/20240731043835.1828697-1-yangerkun@huawei.com Reviewed-by: Chuck Lever chuck.lever@oracle.com [brauner: only update last_index after seek when offset is zero like Jan suggested] Signed-off-by: Christian Brauner brauner@kernel.org Signed-off-by: Andrea Ciprietti ciprietti@google.com

---

fs/libfs.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c index dc0f7519045f..916c39e758b1 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -371,6 +371,15 @@ void simple_offset_destroy(struct offset_ctx *octx) xa_destroy(&octx->xa); } +static int offset_dir_open(struct inode *inode, struct file *file) +{

• struct offset_ctx *ctx = inode->i_op->get_offset_ctx(inode);
• unsigned long next_offset = (unsigned long)ctx->next_offset;
• file->private_data = (void *)next_offset;
• return 0;

+}

• /**
  • offset_dir_llseek - Advance the read position of a directory descriptor
  • @file: an open directory whose position is to be updated

@@ -384,6 +393,9 @@ void simple_offset_destroy(struct offset_ctx *octx) */ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence) {

• struct inode *inode = file->f_inode;
• struct offset_ctx *ctx = inode->i_op->get_offset_ctx(inode);
• switch (whence) { case SEEK_CUR: offset += file->f_pos;

@@ -397,7 +409,11 @@ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence) } /* In this case, ->private_data is protected by f_pos_lock */

• file->private_data = NULL;

• if (!offset) {

• unsigned long next_offset = (unsigned long)ctx->next_offset;
  

• file->private_data = (void *)next_offset;
  

• } return vfs_setpos(file, offset, U32_MAX); }

@@ -427,7 +443,7 @@ static bool offset_dir_emit(struct dir_context *ctx, struct dentry *dentry) inode->i_ino, fs_umode_to_dtype(inode->i_mode)); } -static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) +static void offset_iterate_dir(struct inode *inode, struct dir_context *ctx, long last_index) { struct offset_ctx *so_ctx = inode->i_op->get_offset_ctx(inode); XA_STATE(xas, &so_ctx->xa, ctx->pos); @@ -436,17 +452,21 @@ static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) while (true) { dentry = offset_find_next(&xas); if (!dentry)

• 	return ERR_PTR(-ENOENT);
  

• 	return;
  

• if (dentry2offset(dentry) >= last_index) {
  

• 	dput(dentry);
  

• 	return;
  

• }
  

if (!offset_dir_emit(ctx, dentry)) { dput(dentry);

• 	break;
  

• 	return;
  

  }

dput(dentry); ctx->pos = xas.xa_index + 1; }

• return NULL; }

/** @@ -473,22 +493,19 @@ static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) static int offset_readdir(struct file *file, struct dir_context *ctx) { struct dentry *dir = file->f_path.dentry;

• long last_index = (long)file->private_data;

lockdep_assert_held(&d_inode(dir)->i_rwsem); if (!dir_emit_dots(file, ctx)) return 0;

• /* In this case, ->private_data is protected by f_pos_lock */
• if (ctx->pos == 2)

• file->private_data = NULL;
  

• else if (file->private_data == ERR_PTR(-ENOENT))

• return 0;
  

• file->private_data = offset_iterate_dir(d_inode(dir), ctx);

• offset_iterate_dir(d_inode(dir), ctx, last_index); return 0; }

const struct file_operations simple_offset_dir_operations = {

• .open = offset_dir_open, .llseek = offset_dir_llseek, .iterate_shared = offset_readdir, .read = generic_read_dir,

That commit has some other problems, so we're currently considering this series instead:

https://lore.kernel.org/linux-fsdevel/20250124191946.22308-1-cel@kernel.org/