* getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold @ 2019-11-20 0:15 Rich Felker 2019-11-20 19:57 ` [musl] " Florian Weimer 0 siblings, 1 reply; 6+ messages in thread From: Rich Felker @ 2019-11-20 0:15 UTC (permalink / raw) To: linux-fsdevel; +Cc: musl, linux-kernel, linux-nfs, linux-cifs An issue was reported today on the Alpine Linux tracker at https://gitlab.alpinelinux.org/alpine/aports/issues/10960 regarding readdir results from SMB/NFS shares with musl libc. After a good deal of analysis, we determined the root cause to be that the second and subsequent calls to getdents64 are dropping/skipping direntries (that have not yet been deleted) when some entries were deleted following the previous call. The issue appears to happen only when the buffer size passed to getdents64 is below some threshold greater than 2k (the size musl uses) but less than 32k (the size glibc uses, with which we were unable to reproduce the issue). My guess at the mechanism of failure is that the kernel has cached some entries which it obtained from the FS server based on whatever its preferred transfer size is, but didn't yet pass them to userspace due to limited buffer space, and then purged the buffer when resuming getdents64 after some entries were deleted for reasons related to the changes made way back in 0c0308066ca5 (NFS: Fix spurious readdir cookie loop messages). If so, any such purge likely needs to be delayed until already-buffered results are read, and there may be related buggy interactions with seeking that need to be examined. The to/cc for this message are just my best guesses. Please cc anyone I missed who should be included when replying, and keep me on cc since I'm not subscribed to any of these lists but the musl one. Rich ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold 2019-11-20 0:15 getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold Rich Felker @ 2019-11-20 19:57 ` Florian Weimer 2019-11-20 20:59 ` Rich Felker 0 siblings, 1 reply; 6+ messages in thread From: Florian Weimer @ 2019-11-20 19:57 UTC (permalink / raw) To: Rich Felker; +Cc: linux-fsdevel, musl, linux-kernel, linux-nfs, linux-cifs * Rich Felker: > An issue was reported today on the Alpine Linux tracker at > https://gitlab.alpinelinux.org/alpine/aports/issues/10960 regarding > readdir results from SMB/NFS shares with musl libc. > > After a good deal of analysis, we determined the root cause to be that > the second and subsequent calls to getdents64 are dropping/skipping > direntries (that have not yet been deleted) when some entries were > deleted following the previous call. The issue appears to happen only > when the buffer size passed to getdents64 is below some threshold > greater than 2k (the size musl uses) but less than 32k (the size glibc > uses, with which we were unable to reproduce the issue). From the Gitlab issue: while ((dp = readdir(dir)) != NULL) { unlink(dp->d_name); ++file_cnt; } I'm not sure that this is valid code to delete the contents of a directory. It's true that POSIX says this: | 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. But many file systems simply provide not the necessary on-disk data structures which are need to ensure stable iteration in the face of modification of the directory. There are hacks, of course, such as compacting the on-disk directory only on file creation, which solves the file removal case. For deleting an entire directory, that is not really a problem because you can stick another loop around this while loop which re-reads the directory after rewinddir. Eventually, it will become empty. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold 2019-11-20 19:57 ` [musl] " Florian Weimer @ 2019-11-20 20:59 ` Rich Felker 2019-11-21 17:54 ` Theodore Y. Ts'o 0 siblings, 1 reply; 6+ messages in thread From: Rich Felker @ 2019-11-20 20:59 UTC (permalink / raw) To: Florian Weimer; +Cc: linux-fsdevel, musl, linux-kernel, linux-nfs, linux-cifs On Wed, Nov 20, 2019 at 08:57:32PM +0100, Florian Weimer wrote: > * Rich Felker: > > > An issue was reported today on the Alpine Linux tracker at > > https://gitlab.alpinelinux.org/alpine/aports/issues/10960 regarding > > readdir results from SMB/NFS shares with musl libc. > > > > After a good deal of analysis, we determined the root cause to be that > > the second and subsequent calls to getdents64 are dropping/skipping > > direntries (that have not yet been deleted) when some entries were > > deleted following the previous call. The issue appears to happen only > > when the buffer size passed to getdents64 is below some threshold > > greater than 2k (the size musl uses) but less than 32k (the size glibc > > uses, with which we were unable to reproduce the issue). > > >From the Gitlab issue: > > while ((dp = readdir(dir)) != NULL) { > unlink(dp->d_name); > ++file_cnt; > } > > I'm not sure that this is valid code to delete the contents of a > directory. It's true that POSIX says this: I think it is. > | 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. ^^^^^^^^^^^^^ POSIX only allows both behaviors (showing or not showing) the entry that was deleted. It does not allow deletion of one entry to cause other entries not to be seen. > But many file systems simply provide not the necessary on-disk data > structures which are need to ensure stable iteration in the face of > modification of the directory. There are hacks, of course, such as > compacting the on-disk directory only on file creation, which solves > the file removal case. > > For deleting an entire directory, that is not really a problem because > you can stick another loop around this while loop which re-reads the > directory after rewinddir. Eventually, it will become empty. This is still a serious problem and affects usage other than deletion of an entire directory. Rich ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold 2019-11-20 20:59 ` Rich Felker @ 2019-11-21 17:54 ` Theodore Y. Ts'o 2019-12-25 19:38 ` Florian Weimer 0 siblings, 1 reply; 6+ messages in thread From: Theodore Y. Ts'o @ 2019-11-21 17:54 UTC (permalink / raw) To: Rich Felker Cc: Florian Weimer, linux-fsdevel, musl, linux-kernel, linux-nfs, linux-cifs On Wed, Nov 20, 2019 at 03:59:13PM -0500, Rich Felker wrote: > > POSIX only allows both behaviors (showing or not showing) the entry > that was deleted. It does not allow deletion of one entry to cause > other entries not to be seen. Agreed, but POSIX requires this of *readdir*. POSIX says nothing about getdents64(2), which is Linux's internal implementation which is exposed to a libc. So we would need to see what is exactly going on at the interfaces between the VFS and libc, the nfs client code and the VFS, the nfs client code and the nfs server, and possibly the behavior of the nfs server. First of all.... you can't reproduce this on anything other than with NFS, correct? That is, does it show up if you are using ext4, xfs, btrfs, etc.? Secondly, have you tried this on more than one NFS server implementation? Finally, can you capture strace logs and tcpdump logs of the communication between the NFS client and server code? > > But many file systems simply provide not the necessary on-disk data > > structures which are need to ensure stable iteration in the face of > > modification of the directory. There are hacks, of course, such as > > compacting the on-disk directory only on file creation, which solves > > the file removal case. Oh, that's not the worst of it. You have to do a lot more if the file system needs to support telldir/seekdir, and if you want to export the file system over NFS. If you are using anything other than a linear linked list implementation for your directory, you have to really turn sommersaults to make sure things work (and work efficiently) in the face of, say, node splits of you are using some kind of tree structure for your directory. Most file systems do get this right, at least if they hope to be safely able to be exportable via NFS, or via CIFS using Samba. - Ted ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [musl] getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold 2019-11-21 17:54 ` Theodore Y. Ts'o @ 2019-12-25 19:38 ` Florian Weimer 2019-12-26 3:56 ` Theodore Y. Ts'o 0 siblings, 1 reply; 6+ messages in thread From: Florian Weimer @ 2019-12-25 19:38 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: Rich Felker, linux-fsdevel, musl, linux-kernel, linux-nfs, linux-cifs * Theodore Y. Ts'o: > On Wed, Nov 20, 2019 at 03:59:13PM -0500, Rich Felker wrote: >> >> POSIX only allows both behaviors (showing or not showing) the entry >> that was deleted. It does not allow deletion of one entry to cause >> other entries not to be seen. > > Agreed, but POSIX requires this of *readdir*. POSIX says nothing > about getdents64(2), which is Linux's internal implementation which is > exposed to a libc. Sure, but Linux better provides some reasonable foundation for a libc. I mean, sure, we can read the entire directory into RAM on the first readdir, and get a fully conforming implementation this way (and as Rich noted, glibc's 32 KiB buffer tends to approximate that in practice). But that doesn't strike me as particularly useful. The POSIX requirement is really unfortunate because it leads to incorrect implementations of rm -rf which would on a compliant system and fail in practice. > So we would need to see what is exactly going on at the interfaces > between the VFS and libc, the nfs client code and the VFS, the nfs > client code and the nfs server, and possibly the behavior of the nfs > server. > > First of all.... you can't reproduce this on anything other than with > NFS, correct? That is, does it show up if you are using ext4, xfs, > btrfs, etc.? I'm sure it shows up with certain directory contents on any Linux file system except for those that happen to have a separate B-tree (or equivalent) for telldir/seekdir support. And even those will have broken corner case in case of billions of directory operations. 32 bits are simply not enough storage space for the cookie. Hashing just masks the presence of these bugs, but does not eliminate them completely. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold 2019-12-25 19:38 ` Florian Weimer @ 2019-12-26 3:56 ` Theodore Y. Ts'o 0 siblings, 0 replies; 6+ messages in thread From: Theodore Y. Ts'o @ 2019-12-26 3:56 UTC (permalink / raw) To: Florian Weimer Cc: Rich Felker, linux-fsdevel, musl, linux-kernel, linux-nfs, linux-cifs On Wed, Dec 25, 2019 at 08:38:07PM +0100, Florian Weimer wrote: > 32 bits are simply not enough storage space for the cookie. Hashing > just masks the presence of these bugs, but does not eliminate them > completely. Arguably 64 bits is not enough space for the cookie. I'd be a lot happier if it was 128 or 256 bits. This is just one of those places where POSIX is Really Broken(tm). Unfortunately, NFS only gives us 64 bits for the readdir/readdirplus cookie, so we're kind of stuck with it. - Ted ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-26 3:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-20 0:15 getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold Rich Felker 2019-11-20 19:57 ` [musl] " Florian Weimer 2019-11-20 20:59 ` Rich Felker 2019-11-21 17:54 ` Theodore Y. Ts'o 2019-12-25 19:38 ` Florian Weimer 2019-12-26 3:56 ` Theodore Y. Ts'o
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).