opendir()/readdir() bug in smbd

[Ralph Boehme]

For true concurrency there will always be a race and afaict true 
concurrency is actually not the problematic scenario.

The problematic scenario seems to be an application bug in our torture 
test doing the following sequence of operations:

- open directory
- create "file" in directory
- list directory and expect created file to be present in the list of files

This is an application bug, because as specified by POSIX, it is 
undefined whether "file" is returned.

While adding a rewinddir() would fix the application problem, I'm 
reluctant to changing our server code in a way that would at least 
require tracking of the state of directory fsps and whether an incoming 
list request is the first one or not.

So what about just keeping the application fixed and moving on?

Thanks for looking into this!
-slow


On 12/20/24 11:51 AM, Andreas Schneider via samba-technical wrote:
> Hi,
> 
> we have a bug in Samba which I discovered by running tests on btrfs.
> 
> The POSIX specification for readdir/opendir is the following:
> 
>    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**.
> 
> See https://pubs.opengroup.org/onlinepubs/9799919799/functions/readdir.html
> 
> ext4:
> 
> $ ./repro
> opendir(foo)
> creat(foo/bar)
> readdir() loop
>    readdir entry: .
>    readdir entry: ..
>    readdir entry: bar
> readdir() detected the newly created file foo/bar
> 
> 
> btrfs:
> 
> $ ./repro
> opendir(foo)
> creat(foo/bar)
> readdir() loop
>    readdir entry: .
>    readdir entry: ..
> readdir() did NOT detect the newly created file foo/bar
> 
> 
> I've discussed this with our Kernel people and it is a bug in Samba, we have
> to call rewinddir().
> 
> 
> btrfs:
> 
> $ ./repro rewind
> opendir(foo)
> creat(foo/bar)
> rewinddir(foo)
> readdir() loop
>    readdir entry: .
>    readdir entry: ..
>    readdir entry: bar
> readdir() detected the newly created file foo/bar
> 
> 
> I'm not a file server expert, so I would appreciate if someone could look into
> that.
> 
> Either we call rewinddir() after a file has been created or deleted, or we
> call it before a directory traversal is started. rewinddir() is actually doing
> an lseek().
> 
> The following commits should be reverted once we have a fix:
> 
> fe96aa111cdcc0f753075cccb8f1fd44791abaab
> 38b8a4f2232ca3e8dc4596c080df770f07fa49a8
> 
> 
> 
> Best regards
> 
> 
> 	Andreas
> 
> P.S. tmpfs also behaves like btrfs
> 
>