* [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename()
[not found] <20211021151528.116818-1-lmb@cloudflare.com>
@ 2021-10-21 15:15 ` Lorenz Bauer
2021-10-22 0:34 ` Alexei Starovoitov
2021-10-27 23:21 ` Daniel Borkmann
2021-10-21 15:15 ` [PATCH bpf-next v2 2/3] selftests: bpf: convert test_bpffs to ASSERT macros Lorenz Bauer
2021-10-21 15:15 ` [PATCH bpf-next v2 3/3] selftests: bpf: test RENAME_EXCHANGE and RENAME_NOREPLACE on bpffs Lorenz Bauer
2 siblings, 2 replies; 7+ messages in thread
From: Lorenz Bauer @ 2021-10-21 15:15 UTC (permalink / raw)
To: Alexander Viro, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: kernel-team, Lorenz Bauer, linux-fsdevel, linux-kernel, netdev, bpf
Allow atomic exchange via RENAME_EXCHANGE when using simple_rename.
This affects binderfs, ramfs, hubetlbfs and bpffs. There isn't much
to do except update the various *time fields.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
fs/libfs.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index 51b4de3b3447..93c03d593749 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -455,9 +455,12 @@ int simple_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
struct inode *inode = d_inode(old_dentry);
int they_are_dirs = d_is_dir(old_dentry);
- if (flags & ~RENAME_NOREPLACE)
+ if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE))
return -EINVAL;
+ if (flags & RENAME_EXCHANGE)
+ goto done;
+
if (!simple_empty(new_dentry))
return -ENOTEMPTY;
@@ -472,6 +475,7 @@ int simple_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
inc_nlink(new_dir);
}
+done:
old_dir->i_ctime = old_dir->i_mtime = new_dir->i_ctime =
new_dir->i_mtime = inode->i_ctime = current_time(old_dir);
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename()
2021-10-21 15:15 ` [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename() Lorenz Bauer
@ 2021-10-22 0:34 ` Alexei Starovoitov
2021-10-27 23:21 ` Daniel Borkmann
1 sibling, 0 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2021-10-22 0:34 UTC (permalink / raw)
To: Lorenz Bauer
Cc: Alexander Viro, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, kernel-team, Linux-Fsdevel, LKML,
Network Development, bpf
On Thu, Oct 21, 2021 at 8:16 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Allow atomic exchange via RENAME_EXCHANGE when using simple_rename.
> This affects binderfs, ramfs, hubetlbfs and bpffs. There isn't much
> to do except update the various *time fields.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
Al,
could you please Ack this patch so we can route the whole set through
bpf-next tree?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename()
2021-10-21 15:15 ` [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename() Lorenz Bauer
2021-10-22 0:34 ` Alexei Starovoitov
@ 2021-10-27 23:21 ` Daniel Borkmann
2021-10-28 8:43 ` Miklos Szeredi
1 sibling, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2021-10-27 23:21 UTC (permalink / raw)
To: Lorenz Bauer, Alexander Viro, Alexei Starovoitov, Andrii Nakryiko
Cc: kernel-team, linux-fsdevel, linux-kernel, netdev, bpf, mszeredi, gregkh
[ Adding Miklos & Greg to Cc for review given e0e0be8a8355 ("libfs: support RENAME_NOREPLACE in
simple_rename()"). If you have a chance, would be great if you could take a look, thanks! ]
On 10/21/21 5:15 PM, Lorenz Bauer wrote:
> Allow atomic exchange via RENAME_EXCHANGE when using simple_rename.
> This affects binderfs, ramfs, hubetlbfs and bpffs. There isn't much
> to do except update the various *time fields.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
> fs/libfs.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 51b4de3b3447..93c03d593749 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -455,9 +455,12 @@ int simple_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
> struct inode *inode = d_inode(old_dentry);
> int they_are_dirs = d_is_dir(old_dentry);
>
> - if (flags & ~RENAME_NOREPLACE)
> + if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE))
> return -EINVAL;
>
> + if (flags & RENAME_EXCHANGE)
> + goto done;
> +
> if (!simple_empty(new_dentry))
> return -ENOTEMPTY;
>
> @@ -472,6 +475,7 @@ int simple_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
> inc_nlink(new_dir);
> }
>
> +done:
> old_dir->i_ctime = old_dir->i_mtime = new_dir->i_ctime =
> new_dir->i_mtime = inode->i_ctime = current_time(old_dir);
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename()
2021-10-27 23:21 ` Daniel Borkmann
@ 2021-10-28 8:43 ` Miklos Szeredi
2021-10-28 9:49 ` Lorenz Bauer
0 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2021-10-28 8:43 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Lorenz Bauer, Alexander Viro, Alexei Starovoitov,
Andrii Nakryiko, kernel-team, linux-fsdevel, lkml, netdev, bpf,
Greg Kroah-Hartman
On Thu, Oct 28, 2021 at 1:46 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> [ Adding Miklos & Greg to Cc for review given e0e0be8a8355 ("libfs: support RENAME_NOREPLACE in
> simple_rename()"). If you have a chance, would be great if you could take a look, thanks! ]
>
> On 10/21/21 5:15 PM, Lorenz Bauer wrote:
> > Allow atomic exchange via RENAME_EXCHANGE when using simple_rename.
> > This affects binderfs, ramfs, hubetlbfs and bpffs. There isn't much
> > to do except update the various *time fields.
> >
> > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> > ---
> > fs/libfs.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/libfs.c b/fs/libfs.c
> > index 51b4de3b3447..93c03d593749 100644
> > --- a/fs/libfs.c
> > +++ b/fs/libfs.c
> > @@ -455,9 +455,12 @@ int simple_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
> > struct inode *inode = d_inode(old_dentry);
> > int they_are_dirs = d_is_dir(old_dentry);
> >
> > - if (flags & ~RENAME_NOREPLACE)
> > + if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE))
> > return -EINVAL;
> >
> > + if (flags & RENAME_EXCHANGE)
> > + goto done;
> > +
This is not sufficient. RENAME_EXCHANGE can swap a dir and a
non-dir, in which case the parent nlink counters need to be fixed up.
See shmem_exchange(). My suggestion is to move that function to
libfs.c:simple_rename_exchange().
Thanks,
Miklos
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename()
2021-10-28 8:43 ` Miklos Szeredi
@ 2021-10-28 9:49 ` Lorenz Bauer
0 siblings, 0 replies; 7+ messages in thread
From: Lorenz Bauer @ 2021-10-28 9:49 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Daniel Borkmann, Alexander Viro, Alexei Starovoitov,
Andrii Nakryiko, kernel-team, linux-fsdevel, lkml, Networking,
bpf, Greg Kroah-Hartman
On Thu, 28 Oct 2021 at 09:43, Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> This is not sufficient. RENAME_EXCHANGE can swap a dir and a
> non-dir, in which case the parent nlink counters need to be fixed up.
>
> See shmem_exchange(). My suggestion is to move that function to
> libfs.c:simple_rename_exchange().
Thanks for the pointer, I sent a v3.
Lorenz
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 2/3] selftests: bpf: convert test_bpffs to ASSERT macros
[not found] <20211021151528.116818-1-lmb@cloudflare.com>
2021-10-21 15:15 ` [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename() Lorenz Bauer
@ 2021-10-21 15:15 ` Lorenz Bauer
2021-10-21 15:15 ` [PATCH bpf-next v2 3/3] selftests: bpf: test RENAME_EXCHANGE and RENAME_NOREPLACE on bpffs Lorenz Bauer
2 siblings, 0 replies; 7+ messages in thread
From: Lorenz Bauer @ 2021-10-21 15:15 UTC (permalink / raw)
To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: kernel-team, Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel
Remove usage of deprecated CHECK macros.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
.../selftests/bpf/prog_tests/test_bpffs.c | 22 +++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
index 172c999e523c..533e3f3a459a 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
@@ -29,43 +29,43 @@ static int read_iter(char *file)
static int fn(void)
{
- int err, duration = 0;
+ int err;
err = unshare(CLONE_NEWNS);
- if (CHECK(err, "unshare", "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "unshare"))
goto out;
err = mount("", "/", "", MS_REC | MS_PRIVATE, NULL);
- if (CHECK(err, "mount /", "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "mount /"))
goto out;
err = umount(TDIR);
- if (CHECK(err, "umount " TDIR, "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "umount " TDIR))
goto out;
err = mount("none", TDIR, "tmpfs", 0, NULL);
- if (CHECK(err, "mount", "mount root failed: %d\n", errno))
+ if (!ASSERT_OK(err, "mount tmpfs"))
goto out;
err = mkdir(TDIR "/fs1", 0777);
- if (CHECK(err, "mkdir "TDIR"/fs1", "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "mkdir " TDIR "/fs1"))
goto out;
err = mkdir(TDIR "/fs2", 0777);
- if (CHECK(err, "mkdir "TDIR"/fs2", "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "mkdir " TDIR "/fs2"))
goto out;
err = mount("bpf", TDIR "/fs1", "bpf", 0, NULL);
- if (CHECK(err, "mount bpffs "TDIR"/fs1", "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "mount bpffs " TDIR "/fs1"))
goto out;
err = mount("bpf", TDIR "/fs2", "bpf", 0, NULL);
- if (CHECK(err, "mount bpffs " TDIR "/fs2", "failed: %d\n", errno))
+ if (!ASSERT_OK(err, "mount bpffs " TDIR "/fs2"))
goto out;
err = read_iter(TDIR "/fs1/maps.debug");
- if (CHECK(err, "reading " TDIR "/fs1/maps.debug", "failed\n"))
+ if (!ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug"))
goto out;
err = read_iter(TDIR "/fs2/progs.debug");
- if (CHECK(err, "reading " TDIR "/fs2/progs.debug", "failed\n"))
+ if (!ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug"))
goto out;
out:
umount(TDIR "/fs1");
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH bpf-next v2 3/3] selftests: bpf: test RENAME_EXCHANGE and RENAME_NOREPLACE on bpffs
[not found] <20211021151528.116818-1-lmb@cloudflare.com>
2021-10-21 15:15 ` [PATCH bpf-next v2 1/3] libfs: support RENAME_EXCHANGE in simple_rename() Lorenz Bauer
2021-10-21 15:15 ` [PATCH bpf-next v2 2/3] selftests: bpf: convert test_bpffs to ASSERT macros Lorenz Bauer
@ 2021-10-21 15:15 ` Lorenz Bauer
2 siblings, 0 replies; 7+ messages in thread
From: Lorenz Bauer @ 2021-10-21 15:15 UTC (permalink / raw)
To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: kernel-team, Lorenz Bauer, linux-kselftest, netdev, bpf, linux-kernel
Add tests to exercise the behaviour of RENAME_EXCHANGE and RENAME_NOREPLACE
on bpffs. The former checks that after an exchange the inode of two
directories has changed. The latter checks that the source still exists
after a failed rename.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
.../selftests/bpf/prog_tests/test_bpffs.c | 65 ++++++++++++++++++-
1 file changed, 64 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
index 533e3f3a459a..d29ebfeef9c5 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2020 Facebook */
#define _GNU_SOURCE
+#include <stdio.h>
#include <sched.h>
#include <sys/mount.h>
#include <sys/stat.h>
@@ -29,7 +30,8 @@ static int read_iter(char *file)
static int fn(void)
{
- int err;
+ struct stat a, b, c;
+ int err, map;
err = unshare(CLONE_NEWNS);
if (!ASSERT_OK(err, "unshare"))
@@ -67,6 +69,67 @@ static int fn(void)
err = read_iter(TDIR "/fs2/progs.debug");
if (!ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug"))
goto out;
+
+ err = mkdir(TDIR "/fs1/a", 0777);
+ if (!ASSERT_OK(err, "creating " TDIR "/fs1/a"))
+ goto out;
+ err = mkdir(TDIR "/fs1/a/1", 0777);
+ if (!ASSERT_OK(err, "creating " TDIR "/fs1/a/1"))
+ goto out;
+ err = mkdir(TDIR "/fs1/b", 0777);
+ if (!ASSERT_OK(err, "creating " TDIR "/fs1/b"))
+ goto out;
+
+ map = bpf_create_map(BPF_MAP_TYPE_ARRAY, 4, 4, 1, 0);
+ if (!ASSERT_GT(map, 0, "create_map(ARRAY)"))
+ goto out;
+ err = bpf_obj_pin(map, TDIR "/fs1/c");
+ if (!ASSERT_OK(err, "pin map"))
+ goto out;
+ close(map);
+
+ /* Check that RENAME_EXCHANGE works for directories. */
+ err = stat(TDIR "/fs1/a", &a);
+ if (!ASSERT_OK(err, "stat(" TDIR "/fs1/a)"))
+ goto out;
+ err = renameat2(0, TDIR "/fs1/a", 0, TDIR "/fs1/b", RENAME_EXCHANGE);
+ if (!ASSERT_OK(err, "renameat2(/fs1/a, /fs1/b, RENAME_EXCHANGE)"))
+ goto out;
+ err = stat(TDIR "/fs1/b", &b);
+ if (!ASSERT_OK(err, "stat(" TDIR "/fs1/b)"))
+ goto out;
+ if (!ASSERT_EQ(a.st_ino, b.st_ino, "b should have a's inode"))
+ goto out;
+ err = access(TDIR "/fs1/b/1", F_OK);
+ if (!ASSERT_OK(err, "access(" TDIR "/fs1/b/1)"))
+ goto out;
+
+ /* Check that RENAME_EXCHANGE works for mixed file types. */
+ err = stat(TDIR "/fs1/c", &c);
+ if (!ASSERT_OK(err, "stat(" TDIR "/fs1/map)"))
+ goto out;
+ err = renameat2(0, TDIR "/fs1/c", 0, TDIR "/fs1/b", RENAME_EXCHANGE);
+ if (!ASSERT_OK(err, "renameat2(/fs1/c, /fs1/b, RENAME_EXCHANGE)"))
+ goto out;
+ err = stat(TDIR "/fs1/b", &b);
+ if (!ASSERT_OK(err, "stat(" TDIR "/fs1/b)"))
+ goto out;
+ if (!ASSERT_EQ(c.st_ino, b.st_ino, "b should have c's inode"))
+ goto out;
+ err = access(TDIR "/fs1/c/1", F_OK);
+ if (!ASSERT_OK(err, "access(" TDIR "/fs1/c/1)"))
+ goto out;
+
+ /* Check that RENAME_NOREPLACE works. */
+ err = renameat2(0, TDIR "/fs1/b", 0, TDIR "/fs1/a", RENAME_NOREPLACE);
+ if (!ASSERT_ERR(err, "renameat2(RENAME_NOREPLACE)")) {
+ err = -EINVAL;
+ goto out;
+ }
+ err = access(TDIR "/fs1/b", F_OK);
+ if (!ASSERT_OK(err, "access(" TDIR "/fs1/b)"))
+ goto out;
+
out:
umount(TDIR "/fs1");
umount(TDIR "/fs2");
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread