From: Chuck Lever chuck.lever@oracle.com

[ Upstream commit 3f6d810665dfde0d33785420618ceb03fba0619d ]

Liam and Matthew say that once the RCU read lock is released, xa_state is not safe to re-use for the next xas_find() call. But the RCU read lock must be released on each loop iteration so that dput(), which might_sleep(), can be called safely.

Thus we are forced to walk the offset tree with fresh state for each directory entry. xa_find() can do this for us, though it might be a little less efficient than maintaining xa_state locally.

We believe that in the current code base, inode->i_rwsem provides protection for the xa_state maintained in offset_iterate_dir(). However, there is no guarantee that will continue to be the case in the future.

Since offset_iterate_dir() doesn't build xa_state locally any more, there's no longer a strong need for offset_find_next(). Clean up by rolling these two helpers together.

Suggested-by: Liam R. Howlett Liam.Howlett@Oracle.com Message-ID: 170785993027.11135.8830043889278631735.stgit@91.116.238.104.host.secureserver.net Signed-off-by: Chuck Lever chuck.lever@oracle.com Link: https://lore.kernel.org/r/170820142021.6328.15047865406275957018.stgit@91.11... Reviewed-by: Jan Kara jack@suse.cz Signed-off-by: Christian Brauner brauner@kernel.org Signed-off-by: Chuck Lever chuck.lever@oracle.com --- fs/libfs.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c index dc0f7519045f..430f7c95336c 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -401,12 +401,13 @@ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence) return vfs_setpos(file, offset, U32_MAX); }

-static struct dentry *offset_find_next(struct xa_state *xas) +static struct dentry *offset_find_next(struct offset_ctx *octx, loff_t offset) { struct dentry *child, *found = NULL; + XA_STATE(xas, &octx->xa, offset);

rcu_read_lock(); - child = xas_next_entry(xas, U32_MAX); + child = xas_next_entry(&xas, U32_MAX); if (!child) goto out; spin_lock(&child->d_lock); @@ -429,12 +430,11 @@ static bool offset_dir_emit(struct dir_context *ctx, struct dentry *dentry)

static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) { - struct offset_ctx *so_ctx = inode->i_op->get_offset_ctx(inode); - XA_STATE(xas, &so_ctx->xa, ctx->pos); + struct offset_ctx *octx = inode->i_op->get_offset_ctx(inode); struct dentry *dentry;

while (true) { - dentry = offset_find_next(&xas); + dentry = offset_find_next(octx, ctx->pos); if (!dentry) return ERR_PTR(-ENOENT);

@@ -443,8 +443,8 @@ static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) break; }

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

From: Chuck Lever chuck.lever@oracle.com

[ Upstream commit ecba88a3b32d733d41e27973e25b2bc580f64281 ]

For simple filesystems that use directory offset mapping, rely strictly on the directory offset map to tell when a directory has no children.

After this patch is applied, the emptiness test holds only the RCU read lock when the directory being tested has no children.

In addition, this adds another layer of confirmation that simple_offset_add/remove() are working as expected.

Reviewed-by: Jan Kara jack@suse.cz Signed-off-by: Chuck Lever chuck.lever@oracle.com Link: https://lore.kernel.org/r/170820143463.6328.7872919188371286951.stgit@91.116... Signed-off-by: Christian Brauner brauner@kernel.org Stable-dep-of: 5a1a25be995e ("libfs: Add simple_offset_rename() API") Signed-off-by: Chuck Lever chuck.lever@oracle.com --- fs/libfs.c | 32 ++++++++++++++++++++++++++++++++ include/linux/fs.h | 1 + mm/shmem.c | 4 ++-- 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c index c3dc58e776f9..d7b901cb9af4 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -312,6 +312,38 @@ void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry) offset_set(dentry, 0); }

+/** + * simple_offset_empty - Check if a dentry can be unlinked + * @dentry: dentry to be tested + * + * Returns 0 if @dentry is a non-empty directory; otherwise returns 1. + */ +int simple_offset_empty(struct dentry *dentry) +{ + struct inode *inode = d_inode(dentry); + struct offset_ctx *octx; + struct dentry *child; + unsigned long index; + int ret = 1; + + if (!inode || !S_ISDIR(inode->i_mode)) + return ret; + + index = DIR_OFFSET_MIN; + octx = inode->i_op->get_offset_ctx(inode); + xa_for_each(&octx->xa, index, child) { + spin_lock(&child->d_lock); + if (simple_positive(child)) { + spin_unlock(&child->d_lock); + ret = 0; + break; + } + spin_unlock(&child->d_lock); + } + + return ret; +} + /** * simple_offset_rename_exchange - exchange rename with directory offsets * @old_dir: parent of dentry being moved diff --git a/include/linux/fs.h b/include/linux/fs.h index 6c3d86532e3f..5104405ce3e6 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3197,6 +3197,7 @@ struct offset_ctx { void simple_offset_init(struct offset_ctx *octx); int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry); void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry); +int simple_offset_empty(struct dentry *dentry); int simple_offset_rename_exchange(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, diff --git a/mm/shmem.c b/mm/shmem.c index db7dd45c9181..aaf679976f3b 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3368,7 +3368,7 @@ static int shmem_unlink(struct inode *dir, struct dentry *dentry)

static int shmem_rmdir(struct inode *dir, struct dentry *dentry) { - if (!simple_empty(dentry)) + if (!simple_offset_empty(dentry)) return -ENOTEMPTY;

drop_nlink(d_inode(dentry)); @@ -3425,7 +3425,7 @@ static int shmem_rename2(struct mnt_idmap *idmap, return simple_offset_rename_exchange(old_dir, old_dentry, new_dir, new_dentry);

- if (!simple_empty(new_dentry)) + if (!simple_offset_empty(new_dentry)) return -ENOTEMPTY;

if (flags & RENAME_WHITEOUT) {

From: Chuck Lever chuck.lever@oracle.com

[ Upstream commit 5a1a25be995e1014abd01600479915683e356f5c ]

I'm about to fix a tmpfs rename bug that requires the use of internal simple_offset helpers that are not available in mm/shmem.c

Signed-off-by: Chuck Lever chuck.lever@oracle.com Link: https://lore.kernel.org/r/20240415152057.4605-3-cel@kernel.org Signed-off-by: Christian Brauner brauner@kernel.org Signed-off-by: Chuck Lever chuck.lever@oracle.com --- fs/libfs.c | 21 +++++++++++++++++++++ include/linux/fs.h | 2 ++ mm/shmem.c | 3 +-- 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c index 2029cb6a0e15..15d8c3300dae 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -356,6 +356,27 @@ int simple_offset_empty(struct dentry *dentry) return ret; }

+/** + * simple_offset_rename - handle directory offsets for rename + * @old_dir: parent directory of source entry + * @old_dentry: dentry of source entry + * @new_dir: parent_directory of destination entry + * @new_dentry: dentry of destination + * + * Caller provides appropriate serialization. + * + * Returns zero on success, a negative errno value on failure. + */ +int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry, + struct inode *new_dir, struct dentry *new_dentry) +{ + struct offset_ctx *old_ctx = old_dir->i_op->get_offset_ctx(old_dir); + struct offset_ctx *new_ctx = new_dir->i_op->get_offset_ctx(new_dir); + + simple_offset_remove(old_ctx, old_dentry); + return simple_offset_add(new_ctx, old_dentry); +} + /** * simple_offset_rename_exchange - exchange rename with directory offsets * @old_dir: parent of dentry being moved diff --git a/include/linux/fs.h b/include/linux/fs.h index 5104405ce3e6..e4d139fcaad0 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3198,6 +3198,8 @@ void simple_offset_init(struct offset_ctx *octx); int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry); void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry); int simple_offset_empty(struct dentry *dentry); +int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry, + struct inode *new_dir, struct dentry *new_dentry); int simple_offset_rename_exchange(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, diff --git a/mm/shmem.c b/mm/shmem.c index aaf679976f3b..ab2b0e87b051 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3434,8 +3434,7 @@ static int shmem_rename2(struct mnt_idmap *idmap, return error; }

- simple_offset_remove(shmem_get_offset_ctx(old_dir), old_dentry); - error = simple_offset_add(shmem_get_offset_ctx(new_dir), old_dentry); + error = simple_offset_rename(old_dir, old_dentry, new_dir, new_dentry); if (error) return error;

From: Chuck Lever chuck.lever@oracle.com

[ Upstream commit 903dc9c43a155e0893280c7472d4a9a3a83d75a6 ]

Testing shows that the EBUSY error return from mtree_alloc_cyclic() leaks into user space. The ERRORS section of "man creat(2)" says:

EBUSY O_EXCL was specified in flags and pathname refers to a block device that is in use by the system (e.g., it is mounted).

ENOSPC is closer to what applications expect in this situation.

Note that the normal range of simple directory offset values is 2..2^63, so hitting this error is going to be rare to impossible.

Fixes: 6faddda69f62 ("libfs: Add directory operations for stable offsets") Cc: stable@vger.kernel.org # v6.9+ Reviewed-by: Jeff Layton jlayton@kernel.org Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Chuck Lever chuck.lever@oracle.com Link: https://lore.kernel.org/r/20241228175522.1854234-2-cel@kernel.org Signed-off-by: Christian Brauner brauner@kernel.org [ cel: adjusted to apply to origin/linux-6.6.y ] Signed-off-by: Chuck Lever chuck.lever@oracle.com --- fs/libfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c index ab9fc182fd22..200bcfc2ac34 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -287,8 +287,8 @@ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry)

ret = xa_alloc_cyclic(&octx->xa, &offset, dentry, limit, &octx->next_offset, GFP_KERNEL); - if (ret < 0) - return ret; + if (unlikely(ret < 0)) + return ret == -EBUSY ? -ENOSPC : ret;

offset_set(dentry, offset); return 0;

From: Chuck Lever chuck.lever@oracle.com

[ Upstream commit 68a3a65003145644efcbb651e91db249ccd96281 ]

According to getdents(3), the d_off field in each returned directory entry points to the next entry in the directory. The d_off field in the last returned entry in the readdir buffer must contain a valid offset value, but if it points to an actual directory entry, then readdir/getdents can loop.

This patch introduces a specific fixed offset value that is placed in the d_off field of the last entry in a directory. Some user space applications assume that the EOD offset value is larger than the offsets of real directory entries, so the largest valid offset value is reserved for this purpose. This new value is never allocated by simple_offset_add().

When ->iterate_dir() returns, getdents{64} inserts the ctx->pos value into the d_off field of the last valid entry in the readdir buffer. When it hits EOD, offset_readdir() sets ctx->pos to the EOD offset value so the last entry is updated to point to the EOD marker.

When trying to read the entry at the EOD offset, offset_readdir() terminates immediately.

It is worth noting that using a Maple tree for directory offset value allocation does not guarantee a 63-bit range of values -- on platforms where "long" is a 32-bit type, the directory offset value range is still 0..(2^31 - 1). For broad compatibility with 32-bit user space, the largest tmpfs directory cookie value is now S32_MAX.

Fixes: 796432efab1e ("libfs: getdents() should return 0 after reaching EOD") Signed-off-by: Chuck Lever chuck.lever@oracle.com Link: https://lore.kernel.org/r/20241228175522.1854234-5-cel@kernel.org Signed-off-by: Christian Brauner brauner@kernel.org [ cel: adjusted to apply to origin/linux-6.6.y ] Signed-off-by: Chuck Lever chuck.lever@oracle.com --- fs/libfs.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c index 082bacf0b9e6..d546f3f0c036 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -239,9 +239,15 @@ const struct inode_operations simple_dir_inode_operations = { }; EXPORT_SYMBOL(simple_dir_inode_operations);

-/* 0 is '.', 1 is '..', so always start with offset 2 or more */ +/* simple_offset_add() never assigns these to a dentry */ enum { - DIR_OFFSET_MIN = 2, + DIR_OFFSET_EOD = S32_MAX, +}; + +/* simple_offset_add() allocation range */ +enum { + DIR_OFFSET_MIN = 2, + DIR_OFFSET_MAX = DIR_OFFSET_EOD - 1, };

static void offset_set(struct dentry *dentry, u32 offset) @@ -278,7 +284,8 @@ void simple_offset_init(struct offset_ctx *octx) */ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry) { - static const struct xa_limit limit = XA_LIMIT(DIR_OFFSET_MIN, U32_MAX); + static const struct xa_limit limit = XA_LIMIT(DIR_OFFSET_MIN, + DIR_OFFSET_MAX); u32 offset; int ret;

@@ -442,8 +449,6 @@ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence) return -EINVAL; }

- /* In this case, ->private_data is protected by f_pos_lock */ - file->private_data = NULL; return vfs_setpos(file, offset, U32_MAX); }

@@ -453,7 +458,7 @@ static struct dentry *offset_find_next(struct offset_ctx *octx, loff_t offset) XA_STATE(xas, &octx->xa, offset);

rcu_read_lock(); - child = xas_next_entry(&xas, U32_MAX); + child = xas_next_entry(&xas, DIR_OFFSET_MAX); if (!child) goto out; spin_lock(&child->d_lock); @@ -474,7 +479,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) { struct offset_ctx *octx = inode->i_op->get_offset_ctx(inode); struct dentry *dentry; @@ -482,7 +487,7 @@ static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) while (true) { dentry = offset_find_next(octx, ctx->pos); if (!dentry) - return ERR_PTR(-ENOENT); + goto out_eod;

if (!offset_dir_emit(ctx, dentry)) { dput(dentry); @@ -492,7 +497,10 @@ static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) ctx->pos = dentry2offset(dentry) + 1; dput(dentry); } - return NULL; + return; + +out_eod: + ctx->pos = DIR_OFFSET_EOD; }

/** @@ -512,6 +520,8 @@ static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx) * * On return, @ctx->pos contains an offset that will read the next entry * in this directory when offset_readdir() is called again with @ctx. + * Caller places this value in the d_off field of the last entry in the + * user's buffer. * * Return values: * %0 - Complete @@ -524,13 +534,8 @@ static int offset_readdir(struct file *file, struct dir_context *ctx)

if (!dir_emit_dots(file, ctx)) return 0; - - /* In this case, ->private_data is protected by f_pos_lock */ - if (ctx->pos == DIR_OFFSET_MIN) - file->private_data = NULL; - else if (file->private_data == ERR_PTR(-ENOENT)) - return 0; - file->private_data = offset_iterate_dir(d_inode(dir), ctx); + if (ctx->pos != DIR_OFFSET_EOD) + offset_iterate_dir(d_inode(dir), ctx); return 0; }