Linux-NFS Archive on lore.kernel.org
 help / color / 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; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ 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
  0 siblings, 0 replies; 4+ 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] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ 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

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git