From: David Laight <David.Laight@ACULAB.COM>
To: 'Jan Kara' <jack@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>, Ted Tso <tytso@mit.edu>,
Miklos Szeredi <miklos@szeredi.hu>,
"Darrick J. Wong" <djwong@kernel.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>,
"linux-f2fs-devel@lists.sourceforge.net"
<linux-f2fs-devel@lists.sourceforge.net>,
"linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
Al Viro <viro@ZenIV.linux.org.uk>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
Jaegeuk Kim <jaegeuk@kernel.org>,
"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>
Subject: Re: [f2fs-dev] [PATCH v2 4/6] fs: Establish locking order for unrelated directories
Date: Thu, 1 Jun 2023 16:33:58 +0000 [thread overview]
Message-ID: <eb70760399ae4222904c62c64dc529b6@AcuMS.aculab.com> (raw)
In-Reply-To: <20230601161353.4o6but7hb7i7qfki@quack3>
From: Jan Kara <jack@suse.cz>
> Sent: 01 June 2023 17:14
>
> On Thu 01-06-23 15:37:32, David Laight wrote:
> > ...
> > > > > + * Lock any non-NULL argument. The caller must make sure that if he is passing
> > > > > + * in two directories, one is not ancestor of the other
> >
> > Not directly relevant to this change but is the 'not an ancestor'
> > check actually robust?
> >
> > I found a condition in which the kernel 'pwd' code (which follows
> > the inode chain) failed to stop at the base of a chroot.
> >
> > I suspect that the ancestor check would fail the same way.
>
> Honestly, I'm not sure how this could be the case but I'm not a dcache
> expert. d_ancestor() works on dentries and the whole dcache code pretty
> much relies on the fact that there always is at most one dentry for any
> directory. Also in case we call d_ancestor() from this code, we have the
> whole filesystem locked from any other directory moves so the ancestor
> relationship of two dirs cannot change (which is different from pwd code
> AFAIK). So IMHO no failure is possible in our case.
I've found the test program.
This uses readlinkat() to get the full path /proc/self/fd/0.
It should be inside the chroot, but the comparison done
to detect the 'root' fails.
Now maybe any rename that would hit this is invalid
for other reasons.
But something is awry somewhere.
David
The program below reproduces this when run with stdin
redirected to a file in the current directory.
This sequence is used by 'ip netns exec' so isn't actually
that unusual.
David
#define _GNU_SOURCE
#include <unistd.h>
#include <stdio.h>
#include <fcntl.h>
#include <sched.h>
static void print_link(const char *where, int fd)
{
char buf[256];
printf("%s: %.*s\n", where, (int)readlinkat(fd, "", buf, sizeof buf), buf);
}
int main(int argc, char **argv)
{
int link_fd = open("/proc/self/fd/0", O_PATH | O_NOFOLLOW);
print_link("initial", link_fd);
if (chroot("."))
return 1;
print_link("after chroot", link_fd);
if (unshare(CLONE_NEWNS))
return 2;
print_link("after unshare", link_fd);
return 0;
}
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2023-06-01 16:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-01 10:58 [f2fs-dev] [PATCH v2 0/6] fs: Fix directory corruption when moving directories Jan Kara
2023-06-01 10:58 ` [f2fs-dev] [PATCH v2 1/6] ext4: Remove ext4 locking of moved directory Jan Kara
2023-06-01 14:52 ` Theodore Ts'o
2023-06-01 15:27 ` Jan Kara
2023-06-01 15:58 ` Christian Brauner
2023-06-01 10:58 ` [f2fs-dev] [PATCH v2 2/6] Revert "udf: Protect rename against modification of moved directory" Jan Kara
2023-06-01 10:58 ` [f2fs-dev] [PATCH v2 3/6] Revert "f2fs: fix potential corruption when moving a directory" Jan Kara
2023-06-01 10:58 ` [f2fs-dev] [PATCH v2 4/6] fs: Establish locking order for unrelated directories Jan Kara
2023-06-01 13:58 ` Christian Brauner
2023-06-01 15:24 ` Jan Kara
2023-06-01 15:37 ` David Laight
2023-06-01 16:13 ` Jan Kara
2023-06-01 16:21 ` Christian Brauner
2023-06-01 16:33 ` David Laight [this message]
2023-06-02 12:34 ` Christian Brauner
2023-06-01 15:59 ` Christian Brauner
2023-06-02 1:36 ` kernel test robot
2023-06-01 10:58 ` [f2fs-dev] [PATCH v2 5/6] fs: Lock moved directories Jan Kara
2023-06-01 10:58 ` [f2fs-dev] [PATCH v2 6/6] fs: Restrict lock_two_nondirectories() to non-directory inodes Jan Kara
2023-06-02 13:05 ` [f2fs-dev] [PATCH v2 0/6] fs: Fix directory corruption when moving directories Christian Brauner
2023-07-06 0:18 ` patchwork-bot+f2fs
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=eb70760399ae4222904c62c64dc529b6@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=brauner@kernel.org \
--cc=djwong@kernel.org \
--cc=jack@suse.cz \
--cc=jaegeuk@kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=stable@vger.kernel.org \
--cc=tytso@mit.edu \
--cc=viro@ZenIV.linux.org.uk \
/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.
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).