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

[Jan Kara <jack@suse.cz>]

On Wed 31-07-24 20:51:05, yangerkun wrote:
> 在 2024/7/31 19:51, Jan Kara 写道:
> > On Wed 31-07-24 12:38:35, yangerkun wrote:
> > > 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.
> > 
> > The patch looks good! Just I'm not sure about the llseek part. As far as I
> > understand it was added due to this sentence in the standard:
> > 
> > "If a file is removed from or added to the directory after the most recent
> > call to opendir() or rewinddir(), whether a subsequent call to readdir()
> > returns an entry for that file is unspecified."
> > 
> > So if the offset used in offset_dir_llseek() is 0, then we should update
> > last_index. But otherwise I'd leave it alone because IMHO it would do more
> > harm than good.
> 
> IIUC, what you means is that we should only reset the private_data to
> new last_index when we call rewinddir(which will call lseek to set
> offset of dir file to 0)?

Yes, exactly. Sorry for being a bit vague.

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