Skip to content

Commit b662d85

Browse files
chuckleverbrauner
authored andcommitted
Revert "libfs: fix infinite directory reads for offset dir"
The current directory offset allocator (based on mtree_alloc_cyclic) stores the next offset value to return in octx->next_offset. This mechanism typically returns values that increase monotonically over time. Eventually, though, the newly allocated offset value wraps back to a low number (say, 2) which is smaller than other already- allocated offset values. Yu Kuai <[email protected]> reports that, after commit 64a7ce7 ("libfs: fix infinite directory reads for offset dir"), if a directory's offset allocator wraps, existing entries are no longer visible via readdir/getdents because offset_readdir() stops listing entries once an entry's offset is larger than octx->next_offset. These entries vanish persistently -- they can be looked up, but will never again appear in readdir(3) output. The reason for this is that the commit treats directory offsets as monotonically increasing integer values rather than opaque cookies, and introduces this comparison: if (dentry2offset(dentry) >= last_index) { On 64-bit platforms, the directory offset value upper bound is 2^63 - 1. Directory offsets will monotonically increase for millions of years without wrapping. On 32-bit platforms, however, LONG_MAX is 2^31 - 1. The allocator can wrap after only a few weeks (at worst). Revert commit 64a7ce7 ("libfs: fix infinite directory reads for offset dir") to prepare for a fix that can work properly on 32-bit systems and might apply to recent LTS kernels where shmem employs the simple_offset mechanism. Reported-by: Yu Kuai <[email protected]> Signed-off-by: Chuck Lever <[email protected]> Link: https://lore.kernel.org/r/[email protected] Reviewed-by: Yang Erkun <[email protected]> Signed-off-by: Christian Brauner <[email protected]>
1 parent d7bde4f commit b662d85

File tree

1 file changed

+11
-24
lines changed

1 file changed

+11
-24
lines changed

fs/libfs.c

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -422,14 +422,6 @@ void simple_offset_destroy(struct offset_ctx *octx)
422422
mtree_destroy(&octx->mt);
423423
}
424424

425-
static int offset_dir_open(struct inode *inode, struct file *file)
426-
{
427-
struct offset_ctx *ctx = inode->i_op->get_offset_ctx(inode);
428-
429-
file->private_data = (void *)ctx->next_offset;
430-
return 0;
431-
}
432-
433425
/**
434426
* offset_dir_llseek - Advance the read position of a directory descriptor
435427
* @file: an open directory whose position is to be updated
@@ -443,9 +435,6 @@ static int offset_dir_open(struct inode *inode, struct file *file)
443435
*/
444436
static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence)
445437
{
446-
struct inode *inode = file->f_inode;
447-
struct offset_ctx *ctx = inode->i_op->get_offset_ctx(inode);
448-
449438
switch (whence) {
450439
case SEEK_CUR:
451440
offset += file->f_pos;
@@ -459,8 +448,7 @@ static loff_t offset_dir_llseek(struct file *file, loff_t offset, int whence)
459448
}
460449

461450
/* In this case, ->private_data is protected by f_pos_lock */
462-
if (!offset)
463-
file->private_data = (void *)ctx->next_offset;
451+
file->private_data = NULL;
464452
return vfs_setpos(file, offset, LONG_MAX);
465453
}
466454

@@ -491,29 +479,25 @@ static bool offset_dir_emit(struct dir_context *ctx, struct dentry *dentry)
491479
inode->i_ino, fs_umode_to_dtype(inode->i_mode));
492480
}
493481

494-
static void offset_iterate_dir(struct inode *inode, struct dir_context *ctx, long last_index)
482+
static void *offset_iterate_dir(struct inode *inode, struct dir_context *ctx)
495483
{
496484
struct offset_ctx *octx = inode->i_op->get_offset_ctx(inode);
497485
struct dentry *dentry;
498486

499487
while (true) {
500488
dentry = offset_find_next(octx, ctx->pos);
501489
if (!dentry)
502-
return;
503-
504-
if (dentry2offset(dentry) >= last_index) {
505-
dput(dentry);
506-
return;
507-
}
490+
return ERR_PTR(-ENOENT);
508491

509492
if (!offset_dir_emit(ctx, dentry)) {
510493
dput(dentry);
511-
return;
494+
break;
512495
}
513496

514497
ctx->pos = dentry2offset(dentry) + 1;
515498
dput(dentry);
516499
}
500+
return NULL;
517501
}
518502

519503
/**
@@ -540,19 +524,22 @@ static void offset_iterate_dir(struct inode *inode, struct dir_context *ctx, lon
540524
static int offset_readdir(struct file *file, struct dir_context *ctx)
541525
{
542526
struct dentry *dir = file->f_path.dentry;
543-
long last_index = (long)file->private_data;
544527

545528
lockdep_assert_held(&d_inode(dir)->i_rwsem);
546529

547530
if (!dir_emit_dots(file, ctx))
548531
return 0;
549532

550-
offset_iterate_dir(d_inode(dir), ctx, last_index);
533+
/* In this case, ->private_data is protected by f_pos_lock */
534+
if (ctx->pos == DIR_OFFSET_MIN)
535+
file->private_data = NULL;
536+
else if (file->private_data == ERR_PTR(-ENOENT))
537+
return 0;
538+
file->private_data = offset_iterate_dir(d_inode(dir), ctx);
551539
return 0;
552540
}
553541

554542
const struct file_operations simple_offset_dir_operations = {
555-
.open = offset_dir_open,
556543
.llseek = offset_dir_llseek,
557544
.iterate_shared = offset_readdir,
558545
.read = generic_read_dir,

0 commit comments

Comments
 (0)