From: Rich Felker <dalias@libc.org>
To: Florian Weimer <fw@deneb.enyo.de>
Cc: linux-fsdevel@vger.kernel.org, musl@lists.openwall.com,
linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org,
linux-cifs@vger.kernel.org
Subject: Re: [musl] getdents64 lost direntries with SMB/NFS and buffer size < unknown threshold
Date: Wed, 20 Nov 2019 15:59:13 -0500 [thread overview]
Message-ID: <20191120205913.GD16318@brightrain.aerifal.cx> (raw)
In-Reply-To: <8736eiqq1f.fsf@mid.deneb.enyo.de>
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
next prev parent reply other threads:[~2019-11-20 20:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-20 0:15 Rich Felker
2019-11-20 19:57 ` [musl] " Florian Weimer
2019-11-20 20:59 ` Rich Felker [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191120205913.GD16318@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=fw@deneb.enyo.de \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).