mailing list of musl libc
 help / color / mirror / code / Atom feed
* 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).