mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers
@ 2026-09-20 20:32 Jérémy Jean
  2026-09-20 20:32 ` [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps Jérémy Jean
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jérémy Jean @ 2026-09-20 20:32 UTC (permalink / raw)
  To: miklos, amir73il
  Cc: linux-unionfs, linux-kernel, brauner, Jérémy Jean

Amir, this v2 addresses your comments on v1.

I have split the v2 into 3 commits:

* 1/3 is the actual fix, where only nfs_export is disabled. It caches the
  first lower idmap in each ovl_sb to avoid rescanning all lower layers
  for every repeated superblock.

* 2/3 checks that the targeted layout indeed forces nfs_export=off
  and that decodable handle requests fail with EOPNOTSUPP.

* 3/3 keeps index=on,nfs_export=off and verifies copy up still preserves
  hardlinks and mapped ownership.

I have made this split so that you may decide to include 1 or 2 selftests.

I have reproduced the bug through nfsd with the help of AI. The reproducer
exports an overlay containing a mode 0600 uid 0 file, obtains a real NFSv3
handle, drops a client to uid/gid 1000, then replays the saved handle after
unmount, reclaim, and remount. On an unpatched kernel, the replay
initializes the overlay inode with uid 1000 and the client can read and
overwrite the file. TBH, I don't know whether this layout may actually
happen in production, but it looks like a weird feature.

Changes since v1:
- disable only nfs_export (keep index enabled),
- cache the first lower idmap in struct ovl_sb instead of rescanning,
- add separate selftests for handle rejection and index copy up.

v1: https://lore.kernel.org/all/20260911194201.1334086-2-Jeremy.Jean@oss.cyber.gouv.fr/ 

Jérémy Jean (3):
  ovl: disable nfs_export for same-sb lower layers with different idmaps
  selftests: overlayfs: reject handles for mixed lower idmaps
  selftests: overlayfs: keep index copy up with mixed lower idmaps

 fs/overlayfs/ovl_entry.h                      |   2 +
 fs/overlayfs/super.c                          |  28 ++-
 .../filesystems/overlayfs/idmapped_mounts.c   | 201 +++++++++++++++++-
 3 files changed, 226 insertions(+), 5 deletions(-)

-- 
2.47.3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps
  2026-09-20 20:32 [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Jérémy Jean
@ 2026-09-20 20:32 ` Jérémy Jean
  2026-09-21  8:10   ` Amir Goldstein
  2026-09-20 20:32 ` [PATCH v2 2/3] selftests: overlayfs: reject handles for mixed lower idmaps Jérémy Jean
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Jérémy Jean @ 2026-09-20 20:32 UTC (permalink / raw)
  To: miklos, amir73il
  Cc: linux-unionfs, linux-kernel, brauner, Jérémy Jean

Overlay file handles identify a backing filesystem, but not the lower
mount used to decode it. If lower layers share a superblock and use
different idmaps, a file handle can decode through the wrong layer and
initialize the overlay inode with the wrong owner.

Track the first lower idmap seen for each ovl_sb and disable nfs_export
when another lower layer on the same backing superblock uses a different
idmap. Keep index enabled: copy up starts from the overlay dentry selected
by lookup and does not need to decode a lower file handle through another
layer.

Fixes: bc70682a497c ("ovl: support idmapped layers")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 fs/overlayfs/ovl_entry.h |  2 ++
 fs/overlayfs/super.c     | 28 +++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 80cad4ea96a3..2a6e12a0d4dd 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -24,6 +24,8 @@ struct ovl_config {
 struct ovl_sb {
 	struct super_block *sb;
 	dev_t pseudo_dev;
+	/* Idmap of the first lower layer on this fs */
+	struct mnt_idmap *lower_idmap;
 	/* Unusable (conflicting) uuid */
 	bool bad_uuid;
 	/* Used as a lower layer (but maybe also as upper) */
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index bd0a3f9039d2..8e78da420b75 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -944,6 +944,24 @@ static bool ovl_lower_uuid_ok(struct ovl_fs *ofs, const uuid_t *uuid)
 	return true;
 }
 
+/*
+ * Lower file handles identify a lower fs, but not the mount whose idmap was
+ * used to decode them.  Remember the first lower idmap seen for each fs so
+ * decodable export handles can be disabled if another one appears.
+ */
+static bool ovl_lower_mnt_idmap_mismatch(struct ovl_sb *fs,
+					 const struct path *path)
+{
+	struct mnt_idmap *idmap = mnt_idmap(path->mnt);
+
+	if (!fs->lower_idmap) {
+		fs->lower_idmap = idmap;
+		return false;
+	}
+
+	return fs->lower_idmap != idmap;
+}
+
 /* Get a unique fsid for the layer */
 static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
 {
@@ -956,8 +974,15 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
 	bool warn = false;
 
 	for (i = 0; i < ofs->numfs; i++) {
-		if (ofs->fs[i].sb == sb)
+		if (ofs->fs[i].sb == sb) {
+			if (ofs->config.nfs_export &&
+			    ovl_lower_mnt_idmap_mismatch(&ofs->fs[i], path)) {
+				ofs->config.nfs_export = false;
+				pr_warn("different idmaps in same lower fs '%pd2', falling back to nfs_export=off.\n",
+					path->dentry);
+			}
 			return i;
+		}
 	}
 
 	if (!ovl_lower_uuid_ok(ofs, uuid)) {
@@ -987,6 +1012,7 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
 	ofs->fs[ofs->numfs].sb = sb;
 	ofs->fs[ofs->numfs].pseudo_dev = dev;
 	ofs->fs[ofs->numfs].bad_uuid = bad_uuid;
+	ovl_lower_mnt_idmap_mismatch(&ofs->fs[ofs->numfs], path);
 
 	return ofs->numfs++;
 }
-- 
2.47.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] selftests: overlayfs: reject handles for mixed lower idmaps
  2026-09-20 20:32 [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Jérémy Jean
  2026-09-20 20:32 ` [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps Jérémy Jean
@ 2026-09-20 20:32 ` Jérémy Jean
  2026-09-21  8:14   ` Amir Goldstein
  2026-09-20 20:32 ` [PATCH v2 3/3] selftests: overlayfs: keep index copy up with " Jérémy Jean
  2026-09-21  8:03 ` [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Amir Goldstein
  3 siblings, 1 reply; 9+ messages in thread
From: Jérémy Jean @ 2026-09-20 20:32 UTC (permalink / raw)
  To: miklos, amir73il
  Cc: linux-unionfs, linux-kernel, brauner, Jérémy Jean

Add an idmapped overlay test for two lower layers that share a backing
superblock but use different idmaps. Mount with index=on,nfs_export=on and
verify decodable handle requests fail with EOPNOTSUPP after overlayfs falls
back to nfs_export=off.

Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 .../filesystems/overlayfs/idmapped_mounts.c   | 98 ++++++++++++++++++-
 1 file changed, 94 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
index 44a75839f4ed..0e94bd464f1c 100644
--- a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
+++ b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
@@ -159,8 +159,16 @@ static int idmapped_layer_fd(const char *path, int nsid, int hostid, int range)
 	return fd_tree;
 }
 
-/* Overlay with a layer passed by fd (idmapped) plus a plain upper/work. */
-static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
+static int layer_fd(const char *path)
+{
+	return sys_open_tree(AT_FDCWD, path,
+			     OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
+}
+
+/* Overlay with lower layers passed by fd plus a plain upper/work. */
+static int ovl_mount_lower_fds(const char *upper, const char *work,
+			       int fd_lower1, int fd_lower2,
+			       const char *index, const char *nfs_export)
 {
 	int fsfd, ovl;
 
@@ -171,8 +179,19 @@ static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
 	if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", "test", 0) ||
 	    sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "upperdir", upper, 0) ||
 	    sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "workdir", work, 0) ||
-	    sys_fsconfig(fsfd, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower) ||
-	    sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0))
+	    sys_fsconfig(fsfd, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower1))
+		goto err;
+	if (fd_lower2 >= 0 &&
+	    sys_fsconfig(fsfd, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower2))
+		goto err;
+	if (index &&
+	    sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "index", index, 0))
+		goto err;
+	if (nfs_export &&
+	    sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "nfs_export",
+			 nfs_export, 0))
+		goto err;
+	if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0))
 		goto err;
 
 	ovl = sys_fsmount(fsfd, 0, 0);
@@ -183,6 +202,23 @@ static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
 	return -1;
 }
 
+static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
+{
+	return ovl_mount_lower_fds(upper, work, fd_lower, -1, NULL, NULL);
+}
+
+static int setup_extra_layer(const char *base, const char *name)
+{
+	char path[PATH_MAX];
+
+	snprintf(path, sizeof(path), "%s/%s", base, name);
+	if (mkdir(path, 0755) && errno != EEXIST)
+		return -1;
+	if (chown(path, ID_HOST, ID_HOST))
+		return -1;
+	return 0;
+}
+
 /*
  * Mount an overlay inside user namespace @u1 (so the overlay sb's s_user_ns is
  * not the initial namespace) and idmap that overlay mount with @u2.  Runs in a
@@ -498,4 +534,58 @@ TEST_F(idmapped_overlay, nfs_export_handles)
 	EXPECT_EQ(close(ovl), 0);
 }
 
+/*
+ * Lower file handles cannot be decoded safely when two lower layers share a
+ * superblock but use different idmaps.  nfs_export should be disabled for that
+ * layout, so a normal decodable handle request must fail with EOPNOTSUPP.
+ */
+TEST_F(idmapped_overlay, nfs_export_same_sb_mixed_idmaps)
+{
+	char lower1[PATH_MAX], lower2[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
+	char path[PATH_MAX], mnt[128];
+	union {
+		struct file_handle fh;
+		char buf[sizeof(struct file_handle) + MAX_HANDLE_SZ];
+	} fhu;
+	struct file_handle *fh = &fhu.fh;
+	int fd_lower1, fd_lower2, ovl, mount_id;
+
+	if (!ovl_supported())
+		SKIP(return, "overlayfs not supported");
+
+	snprintf(lower1, sizeof(lower1), "%s/l", self->base);
+	snprintf(lower2, sizeof(lower2), "%s/l2", self->base);
+	snprintf(upper, sizeof(upper), "%s/u", self->base);
+	snprintf(work, sizeof(work), "%s/w", self->base);
+	ASSERT_EQ(setup_extra_layer(self->base, "l2"), 0);
+
+	snprintf(path, sizeof(path), "%s/l2/file", self->base);
+	ASSERT_EQ(mknod(path, S_IFREG | 0644, 0), 0);
+	ASSERT_EQ(chown(path, ID_HOST + 7, ID_HOST + 7), 0);
+
+	fd_lower1 = idmapped_layer_fd(lower1, ID_HOST, ID_NS, ID_RANGE);
+	ASSERT_GE(fd_lower1, 0);
+	fd_lower2 = layer_fd(lower2);
+	ASSERT_GE(fd_lower2, 0);
+
+	ovl = ovl_mount_lower_fds(upper, work, fd_lower1, fd_lower2, "on", "on");
+	if (ovl < 0)
+		SKIP(return, "overlayfs nfs_export not supported");
+	EXPECT_EQ(close(fd_lower1), 0);
+	EXPECT_EQ(close(fd_lower2), 0);
+
+	snprintf(mnt, sizeof(mnt), "%s/mnt", self->base);
+	ASSERT_EQ(mkdir(mnt, 0755), 0);
+	ASSERT_EQ(sys_move_mount(ovl, "", AT_FDCWD, mnt,
+				 MOVE_MOUNT_F_EMPTY_PATH), 0);
+
+	snprintf(path, sizeof(path), "%s/file", mnt);
+	fh->handle_bytes = MAX_HANDLE_SZ;
+	errno = 0;
+	EXPECT_EQ(name_to_handle_at(AT_FDCWD, path, fh, &mount_id, 0), -1);
+	EXPECT_EQ(errno, EOPNOTSUPP);
+
+	EXPECT_EQ(close(ovl), 0);
+}
+
 TEST_HARNESS_MAIN
-- 
2.47.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] selftests: overlayfs: keep index copy up with mixed lower idmaps
  2026-09-20 20:32 [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Jérémy Jean
  2026-09-20 20:32 ` [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps Jérémy Jean
  2026-09-20 20:32 ` [PATCH v2 2/3] selftests: overlayfs: reject handles for mixed lower idmaps Jérémy Jean
@ 2026-09-20 20:32 ` Jérémy Jean
  2026-09-21  8:18   ` Amir Goldstein
  2026-09-21  8:03 ` [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Amir Goldstein
  3 siblings, 1 reply; 9+ messages in thread
From: Jérémy Jean @ 2026-09-20 20:32 UTC (permalink / raw)
  To: miklos, amir73il
  Cc: linux-unionfs, linux-kernel, brauner, Jérémy Jean

Add a copy-up test for mixed-idmap lower layers with
index=on,nfs_export=off.

Write through one hardlink alias and read through the other.
Check that both aliases still point to the same copied-up inode.
Check that the copied-up inode keeps the expected mapped owner.

Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 .../filesystems/overlayfs/idmapped_mounts.c   | 103 ++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
index 0e94bd464f1c..ae99698c280a 100644
--- a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
+++ b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
@@ -219,6 +219,42 @@ static int setup_extra_layer(const char *base, const char *name)
 	return 0;
 }
 
+static int write_path(const char *path, const char *buf)
+{
+	size_t len = strlen(buf);
+	int fd;
+
+	fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
+	if (fd < 0)
+		return -1;
+	if (write(fd, buf, len) != len) {
+		close(fd);
+		return -1;
+	}
+	if (close(fd))
+		return -1;
+	return 0;
+}
+
+static int read_fd_contents(int dirfd, const char *name, char *buf, size_t len)
+{
+	ssize_t n;
+	int fd;
+
+	fd = openat(dirfd, name, O_RDONLY);
+	if (fd < 0)
+		return -1;
+	n = read(fd, buf, len - 1);
+	if (n < 0) {
+		close(fd);
+		return -1;
+	}
+	buf[n] = '\0';
+	if (close(fd))
+		return -1;
+	return 0;
+}
+
 /*
  * Mount an overlay inside user namespace @u1 (so the overlay sb's s_user_ns is
  * not the initial namespace) and idmap that overlay mount with @u2.  Runs in a
@@ -588,4 +624,71 @@ TEST_F(idmapped_overlay, nfs_export_same_sb_mixed_idmaps)
 	EXPECT_EQ(close(ovl), 0);
 }
 
+/*
+ * index=on without nfs_export still preserves lower hardlinks on copy up when
+ * same-superblock lower layers use different idmaps.  The copied-up alias must
+ * keep the same contents, inode identity, and owner as the path-selected lower.
+ */
+TEST_F(idmapped_overlay, index_copy_up_same_sb_mixed_idmaps)
+{
+	static const char lower_data[] = "lower\n";
+	static const char upper_data[] = "upper\n";
+	char lower1[PATH_MAX], lower2[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
+	char path[PATH_MAX], alias[PATH_MAX], buf[32];
+	struct stat st_file, st_alias, st_upper;
+	int fd_lower1, fd_lower2, ovl, fd;
+
+	if (!ovl_supported())
+		SKIP(return, "overlayfs not supported");
+
+	snprintf(lower1, sizeof(lower1), "%s/l", self->base);
+	snprintf(lower2, sizeof(lower2), "%s/l2", self->base);
+	snprintf(upper, sizeof(upper), "%s/u", self->base);
+	snprintf(work, sizeof(work), "%s/w", self->base);
+	ASSERT_EQ(setup_extra_layer(self->base, "l2"), 0);
+
+	snprintf(path, sizeof(path), "%s/l2/file", self->base);
+	snprintf(alias, sizeof(alias), "%s/l2/alias", self->base);
+	ASSERT_EQ(write_path(path, lower_data), 0);
+	ASSERT_EQ(chown(path, ID_HOST + 7, ID_HOST + 7), 0);
+	ASSERT_EQ(link(path, alias), 0);
+
+	fd_lower1 = idmapped_layer_fd(lower1, ID_HOST, ID_NS, ID_RANGE);
+	ASSERT_GE(fd_lower1, 0);
+	fd_lower2 = layer_fd(lower2);
+	ASSERT_GE(fd_lower2, 0);
+
+	ovl = ovl_mount_lower_fds(upper, work, fd_lower1, fd_lower2, "on", "off");
+	ASSERT_GE(ovl, 0);
+	EXPECT_EQ(close(fd_lower1), 0);
+	EXPECT_EQ(close(fd_lower2), 0);
+	ASSERT_EQ(ovl_idmap(ovl), 0);
+
+	fd = openat(ovl, "file", O_WRONLY | O_TRUNC);
+	ASSERT_GE(fd, 0);
+	ASSERT_EQ(write(fd, upper_data, sizeof(upper_data) - 1),
+		  sizeof(upper_data) - 1);
+	EXPECT_EQ(close(fd), 0);
+
+	ASSERT_EQ(read_fd_contents(ovl, "alias", buf, sizeof(buf)), 0);
+	EXPECT_STREQ(upper_data, buf);
+
+	ASSERT_EQ(fstatat(ovl, "file", &st_file, 0), 0);
+	ASSERT_EQ(fstatat(ovl, "alias", &st_alias, 0), 0);
+	EXPECT_EQ(st_file.st_ino, st_alias.st_ino);
+	EXPECT_EQ(st_file.st_nlink, 2);
+	EXPECT_EQ(st_alias.st_nlink, 2);
+	EXPECT_EQ(st_file.st_uid, ID_NS + 7);
+	EXPECT_EQ(st_file.st_gid, ID_NS + 7);
+	EXPECT_EQ(st_alias.st_uid, ID_NS + 7);
+	EXPECT_EQ(st_alias.st_gid, ID_NS + 7);
+
+	snprintf(path, sizeof(path), "%s/u/file", self->base);
+	ASSERT_EQ(stat(path, &st_upper), 0);
+	EXPECT_EQ(st_upper.st_uid, ID_HOST + 7);
+	EXPECT_EQ(st_upper.st_gid, ID_HOST + 7);
+
+	EXPECT_EQ(close(ovl), 0);
+}
+
 TEST_HARNESS_MAIN
-- 
2.47.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers
  2026-09-20 20:32 [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Jérémy Jean
                   ` (2 preceding siblings ...)
  2026-09-20 20:32 ` [PATCH v2 3/3] selftests: overlayfs: keep index copy up with " Jérémy Jean
@ 2026-09-21  8:03 ` Amir Goldstein
  2026-09-21  8:22   ` Jérémy Jean
  3 siblings, 1 reply; 9+ messages in thread
From: Amir Goldstein @ 2026-09-21  8:03 UTC (permalink / raw)
  To: Jérémy Jean, brauner; +Cc: miklos, linux-unionfs, linux-kernel

On Sun, Sep 20, 2026 at 10:33 PM Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>
> Amir, this v2 addresses your comments on v1.
>
> I have split the v2 into 3 commits:
>
> * 1/3 is the actual fix, where only nfs_export is disabled. It caches the
>   first lower idmap in each ovl_sb to avoid rescanning all lower layers
>   for every repeated superblock.
>
> * 2/3 checks that the targeted layout indeed forces nfs_export=off
>   and that decodable handle requests fail with EOPNOTSUPP.
>
> * 3/3 keeps index=on,nfs_export=off and verifies copy up still preserves
>   hardlinks and mapped ownership.
>
> I have made this split so that you may decide to include 1 or 2 selftests.
>
> I have reproduced the bug through nfsd with the help of AI. The reproducer
> exports an overlay containing a mode 0600 uid 0 file, obtains a real NFSv3
> handle, drops a client to uid/gid 1000, then replays the saved handle after
> unmount, reclaim, and remount. On an unpatched kernel, the replay
> initializes the overlay inode with uid 1000 and the client can read and
> overwrite the file. TBH, I don't know whether this layout may actually
> happen in production, but it looks like a weird feature.

I do not know either.

I would like to get feedback from Christian on that, because if this
setup is not expected I'd rather fail the mount.

Thanks,
Amir.

>
> Changes since v1:
> - disable only nfs_export (keep index enabled),
> - cache the first lower idmap in struct ovl_sb instead of rescanning,
> - add separate selftests for handle rejection and index copy up.
>
> v1: https://lore.kernel.org/all/20260911194201.1334086-2-Jeremy.Jean@oss.cyber.gouv.fr/
>
> Jérémy Jean (3):
>   ovl: disable nfs_export for same-sb lower layers with different idmaps
>   selftests: overlayfs: reject handles for mixed lower idmaps
>   selftests: overlayfs: keep index copy up with mixed lower idmaps
>
>  fs/overlayfs/ovl_entry.h                      |   2 +
>  fs/overlayfs/super.c                          |  28 ++-
>  .../filesystems/overlayfs/idmapped_mounts.c   | 201 +++++++++++++++++-
>  3 files changed, 226 insertions(+), 5 deletions(-)
>
> --
> 2.47.3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps
  2026-09-20 20:32 ` [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps Jérémy Jean
@ 2026-09-21  8:10   ` Amir Goldstein
  0 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2026-09-21  8:10 UTC (permalink / raw)
  To: Jérémy Jean; +Cc: miklos, linux-unionfs, linux-kernel, brauner

On Sun, Sep 20, 2026 at 10:33 PM Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>
> Overlay file handles identify a backing filesystem, but not the lower
> mount used to decode it. If lower layers share a superblock and use
> different idmaps, a file handle can decode through the wrong layer and
> initialize the overlay inode with the wrong owner.
>
> Track the first lower idmap seen for each ovl_sb and disable nfs_export
> when another lower layer on the same backing superblock uses a different
> idmap. Keep index enabled: copy up starts from the overlay dentry selected
> by lookup and does not need to decode a lower file handle through another
> layer.
>
> Fixes: bc70682a497c ("ovl: support idmapped layers")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  fs/overlayfs/ovl_entry.h |  2 ++
>  fs/overlayfs/super.c     | 28 +++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index 80cad4ea96a3..2a6e12a0d4dd 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -24,6 +24,8 @@ struct ovl_config {
>  struct ovl_sb {
>         struct super_block *sb;
>         dev_t pseudo_dev;
> +       /* Idmap of the first lower layer on this fs */
> +       struct mnt_idmap *lower_idmap;
>         /* Unusable (conflicting) uuid */
>         bool bad_uuid;
>         /* Used as a lower layer (but maybe also as upper) */
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index bd0a3f9039d2..8e78da420b75 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -944,6 +944,24 @@ static bool ovl_lower_uuid_ok(struct ovl_fs *ofs, const uuid_t *uuid)
>         return true;
>  }
>
> +/*
> + * Lower file handles identify a lower fs, but not the mount whose idmap was
> + * used to decode them.  Remember the first lower idmap seen for each fs so
> + * decodable export handles can be disabled if another one appears.
> + */
> +static bool ovl_lower_mnt_idmap_mismatch(struct ovl_sb *fs,
> +                                        const struct path *path)
> +{
> +       struct mnt_idmap *idmap = mnt_idmap(path->mnt);
> +
> +       if (!fs->lower_idmap) {
> +               fs->lower_idmap = idmap;
> +               return false;
> +       }
> +
> +       return fs->lower_idmap != idmap;
> +}
> +
>  /* Get a unique fsid for the layer */
>  static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
>  {
> @@ -956,8 +974,15 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
>         bool warn = false;
>
>         for (i = 0; i < ofs->numfs; i++) {
> -               if (ofs->fs[i].sb == sb)
> +               if (ofs->fs[i].sb == sb) {
> +                       if (ofs->config.nfs_export &&
> +                           ovl_lower_mnt_idmap_mismatch(&ofs->fs[i], path)) {
> +                               ofs->config.nfs_export = false;
> +                               pr_warn("different idmaps in same lower fs '%pd2', falling back to nfs_export=off.\n",
> +                                       path->dentry);
> +                       }
>                         return i;
> +               }
>         }
>
>         if (!ovl_lower_uuid_ok(ofs, uuid)) {
> @@ -987,6 +1012,7 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
>         ofs->fs[ofs->numfs].sb = sb;
>         ofs->fs[ofs->numfs].pseudo_dev = dev;
>         ofs->fs[ofs->numfs].bad_uuid = bad_uuid;
> +       ovl_lower_mnt_idmap_mismatch(&ofs->fs[ofs->numfs], path);

This helper name spells weird here.
Better just assign lower_idmap directly.

With that fixed, you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

>
>         return ofs->numfs++;
>  }
> --
> 2.47.3
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] selftests: overlayfs: reject handles for mixed lower idmaps
  2026-09-20 20:32 ` [PATCH v2 2/3] selftests: overlayfs: reject handles for mixed lower idmaps Jérémy Jean
@ 2026-09-21  8:14   ` Amir Goldstein
  0 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2026-09-21  8:14 UTC (permalink / raw)
  To: Jérémy Jean, brauner; +Cc: miklos, linux-unionfs, linux-kernel

On Sun, Sep 20, 2026 at 10:33 PM Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>
> Add an idmapped overlay test for two lower layers that share a backing
> superblock but use different idmaps. Mount with index=on,nfs_export=on and
> verify decodable handle requests fail with EOPNOTSUPP after overlayfs falls
> back to nfs_export=off.
>
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  .../filesystems/overlayfs/idmapped_mounts.c   | 98 ++++++++++++++++++-
>  1 file changed, 94 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
> index 44a75839f4ed..0e94bd464f1c 100644
> --- a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
> +++ b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
> @@ -159,8 +159,16 @@ static int idmapped_layer_fd(const char *path, int nsid, int hostid, int range)
>         return fd_tree;
>  }
>
> -/* Overlay with a layer passed by fd (idmapped) plus a plain upper/work. */
> -static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
> +static int layer_fd(const char *path)
> +{
> +       return sys_open_tree(AT_FDCWD, path,
> +                            OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
> +}
> +
> +/* Overlay with lower layers passed by fd plus a plain upper/work. */
> +static int ovl_mount_lower_fds(const char *upper, const char *work,
> +                              int fd_lower1, int fd_lower2,
> +                              const char *index, const char *nfs_export)
>  {
>         int fsfd, ovl;
>
> @@ -171,8 +179,19 @@ static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
>         if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", "test", 0) ||
>             sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "upperdir", upper, 0) ||
>             sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "workdir", work, 0) ||
> -           sys_fsconfig(fsfd, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower) ||
> -           sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0))
> +           sys_fsconfig(fsfd, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower1))
> +               goto err;
> +       if (fd_lower2 >= 0 &&
> +           sys_fsconfig(fsfd, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower2))
> +               goto err;
> +       if (index &&
> +           sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "index", index, 0))
> +               goto err;
> +       if (nfs_export &&
> +           sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "nfs_export",
> +                        nfs_export, 0))
> +               goto err;
> +       if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0))
>                 goto err;
>
>         ovl = sys_fsmount(fsfd, 0, 0);
> @@ -183,6 +202,23 @@ static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
>         return -1;
>  }
>
> +static int ovl_mount_lower_fd(const char *upper, const char *work, int fd_lower)
> +{
> +       return ovl_mount_lower_fds(upper, work, fd_lower, -1, NULL, NULL);
> +}
> +
> +static int setup_extra_layer(const char *base, const char *name)
> +{
> +       char path[PATH_MAX];
> +
> +       snprintf(path, sizeof(path), "%s/%s", base, name);
> +       if (mkdir(path, 0755) && errno != EEXIST)
> +               return -1;
> +       if (chown(path, ID_HOST, ID_HOST))
> +               return -1;
> +       return 0;
> +}
> +
>  /*
>   * Mount an overlay inside user namespace @u1 (so the overlay sb's s_user_ns is
>   * not the initial namespace) and idmap that overlay mount with @u2.  Runs in a
> @@ -498,4 +534,58 @@ TEST_F(idmapped_overlay, nfs_export_handles)
>         EXPECT_EQ(close(ovl), 0);
>  }
>
> +/*
> + * Lower file handles cannot be decoded safely when two lower layers share a
> + * superblock but use different idmaps.  nfs_export should be disabled for that
> + * layout, so a normal decodable handle request must fail with EOPNOTSUPP.
> + */
> +TEST_F(idmapped_overlay, nfs_export_same_sb_mixed_idmaps)
> +{
> +       char lower1[PATH_MAX], lower2[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
> +       char path[PATH_MAX], mnt[128];
> +       union {
> +               struct file_handle fh;
> +               char buf[sizeof(struct file_handle) + MAX_HANDLE_SZ];
> +       } fhu;
> +       struct file_handle *fh = &fhu.fh;
> +       int fd_lower1, fd_lower2, ovl, mount_id;
> +
> +       if (!ovl_supported())
> +               SKIP(return, "overlayfs not supported");
> +
> +       snprintf(lower1, sizeof(lower1), "%s/l", self->base);
> +       snprintf(lower2, sizeof(lower2), "%s/l2", self->base);
> +       snprintf(upper, sizeof(upper), "%s/u", self->base);
> +       snprintf(work, sizeof(work), "%s/w", self->base);
> +       ASSERT_EQ(setup_extra_layer(self->base, "l2"), 0);
> +
> +       snprintf(path, sizeof(path), "%s/l2/file", self->base);
> +       ASSERT_EQ(mknod(path, S_IFREG | 0644, 0), 0);
> +       ASSERT_EQ(chown(path, ID_HOST + 7, ID_HOST + 7), 0);
> +
> +       fd_lower1 = idmapped_layer_fd(lower1, ID_HOST, ID_NS, ID_RANGE);
> +       ASSERT_GE(fd_lower1, 0);
> +       fd_lower2 = layer_fd(lower2);
> +       ASSERT_GE(fd_lower2, 0);
> +
> +       ovl = ovl_mount_lower_fds(upper, work, fd_lower1, fd_lower2, "on", "on");
> +       if (ovl < 0)
> +               SKIP(return, "overlayfs nfs_export not supported");
> +       EXPECT_EQ(close(fd_lower1), 0);
> +       EXPECT_EQ(close(fd_lower2), 0);
> +
> +       snprintf(mnt, sizeof(mnt), "%s/mnt", self->base);
> +       ASSERT_EQ(mkdir(mnt, 0755), 0);
> +       ASSERT_EQ(sys_move_mount(ovl, "", AT_FDCWD, mnt,
> +                                MOVE_MOUNT_F_EMPTY_PATH), 0);
> +
> +       snprintf(path, sizeof(path), "%s/file", mnt);
> +       fh->handle_bytes = MAX_HANDLE_SZ;
> +       errno = 0;
> +       EXPECT_EQ(name_to_handle_at(AT_FDCWD, path, fh, &mount_id, 0), -1);
> +       EXPECT_EQ(errno, EOPNOTSUPP);
> +
> +       EXPECT_EQ(close(ovl), 0);
> +}
> +
>  TEST_HARNESS_MAIN

The expected behavior depends on whether Christian says this
setup is legit in production.

Apart from the that you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] selftests: overlayfs: keep index copy up with mixed lower idmaps
  2026-09-20 20:32 ` [PATCH v2 3/3] selftests: overlayfs: keep index copy up with " Jérémy Jean
@ 2026-09-21  8:18   ` Amir Goldstein
  0 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2026-09-21  8:18 UTC (permalink / raw)
  To: Jérémy Jean, brauner; +Cc: miklos, linux-unionfs, linux-kernel

On Sun, Sep 20, 2026 at 10:33 PM Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>
> Add a copy-up test for mixed-idmap lower layers with
> index=on,nfs_export=off.
>
> Write through one hardlink alias and read through the other.
> Check that both aliases still point to the same copied-up inode.
> Check that the copied-up inode keeps the expected mapped owner.
>
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  .../filesystems/overlayfs/idmapped_mounts.c   | 103 ++++++++++++++++++
>  1 file changed, 103 insertions(+)
>
> diff --git a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
> index 0e94bd464f1c..ae99698c280a 100644
> --- a/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
> +++ b/tools/testing/selftests/filesystems/overlayfs/idmapped_mounts.c
> @@ -219,6 +219,42 @@ static int setup_extra_layer(const char *base, const char *name)
>         return 0;
>  }
>
> +static int write_path(const char *path, const char *buf)
> +{
> +       size_t len = strlen(buf);
> +       int fd;
> +
> +       fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
> +       if (fd < 0)
> +               return -1;
> +       if (write(fd, buf, len) != len) {
> +               close(fd);
> +               return -1;
> +       }
> +       if (close(fd))
> +               return -1;
> +       return 0;
> +}
> +
> +static int read_fd_contents(int dirfd, const char *name, char *buf, size_t len)
> +{
> +       ssize_t n;
> +       int fd;
> +
> +       fd = openat(dirfd, name, O_RDONLY);
> +       if (fd < 0)
> +               return -1;
> +       n = read(fd, buf, len - 1);
> +       if (n < 0) {
> +               close(fd);
> +               return -1;
> +       }
> +       buf[n] = '\0';
> +       if (close(fd))
> +               return -1;
> +       return 0;
> +}
> +
>  /*
>   * Mount an overlay inside user namespace @u1 (so the overlay sb's s_user_ns is
>   * not the initial namespace) and idmap that overlay mount with @u2.  Runs in a
> @@ -588,4 +624,71 @@ TEST_F(idmapped_overlay, nfs_export_same_sb_mixed_idmaps)
>         EXPECT_EQ(close(ovl), 0);
>  }
>
> +/*
> + * index=on without nfs_export still preserves lower hardlinks on copy up when
> + * same-superblock lower layers use different idmaps.  The copied-up alias must
> + * keep the same contents, inode identity, and owner as the path-selected lower.
> + */
> +TEST_F(idmapped_overlay, index_copy_up_same_sb_mixed_idmaps)
> +{
> +       static const char lower_data[] = "lower\n";
> +       static const char upper_data[] = "upper\n";
> +       char lower1[PATH_MAX], lower2[PATH_MAX], upper[PATH_MAX], work[PATH_MAX];
> +       char path[PATH_MAX], alias[PATH_MAX], buf[32];
> +       struct stat st_file, st_alias, st_upper;
> +       int fd_lower1, fd_lower2, ovl, fd;
> +
> +       if (!ovl_supported())
> +               SKIP(return, "overlayfs not supported");
> +
> +       snprintf(lower1, sizeof(lower1), "%s/l", self->base);
> +       snprintf(lower2, sizeof(lower2), "%s/l2", self->base);
> +       snprintf(upper, sizeof(upper), "%s/u", self->base);
> +       snprintf(work, sizeof(work), "%s/w", self->base);
> +       ASSERT_EQ(setup_extra_layer(self->base, "l2"), 0);
> +
> +       snprintf(path, sizeof(path), "%s/l2/file", self->base);
> +       snprintf(alias, sizeof(alias), "%s/l2/alias", self->base);
> +       ASSERT_EQ(write_path(path, lower_data), 0);
> +       ASSERT_EQ(chown(path, ID_HOST + 7, ID_HOST + 7), 0);
> +       ASSERT_EQ(link(path, alias), 0);
> +
> +       fd_lower1 = idmapped_layer_fd(lower1, ID_HOST, ID_NS, ID_RANGE);
> +       ASSERT_GE(fd_lower1, 0);
> +       fd_lower2 = layer_fd(lower2);
> +       ASSERT_GE(fd_lower2, 0);
> +
> +       ovl = ovl_mount_lower_fds(upper, work, fd_lower1, fd_lower2, "on", "off");
> +       ASSERT_GE(ovl, 0);
> +       EXPECT_EQ(close(fd_lower1), 0);
> +       EXPECT_EQ(close(fd_lower2), 0);
> +       ASSERT_EQ(ovl_idmap(ovl), 0);
> +
> +       fd = openat(ovl, "file", O_WRONLY | O_TRUNC);
> +       ASSERT_GE(fd, 0);
> +       ASSERT_EQ(write(fd, upper_data, sizeof(upper_data) - 1),
> +                 sizeof(upper_data) - 1);
> +       EXPECT_EQ(close(fd), 0);
> +
> +       ASSERT_EQ(read_fd_contents(ovl, "alias", buf, sizeof(buf)), 0);
> +       EXPECT_STREQ(upper_data, buf);
> +
> +       ASSERT_EQ(fstatat(ovl, "file", &st_file, 0), 0);
> +       ASSERT_EQ(fstatat(ovl, "alias", &st_alias, 0), 0);
> +       EXPECT_EQ(st_file.st_ino, st_alias.st_ino);
> +       EXPECT_EQ(st_file.st_nlink, 2);
> +       EXPECT_EQ(st_alias.st_nlink, 2);
> +       EXPECT_EQ(st_file.st_uid, ID_NS + 7);
> +       EXPECT_EQ(st_file.st_gid, ID_NS + 7);
> +       EXPECT_EQ(st_alias.st_uid, ID_NS + 7);
> +       EXPECT_EQ(st_alias.st_gid, ID_NS + 7);
> +
> +       snprintf(path, sizeof(path), "%s/u/file", self->base);
> +       ASSERT_EQ(stat(path, &st_upper), 0);
> +       EXPECT_EQ(st_upper.st_uid, ID_HOST + 7);
> +       EXPECT_EQ(st_upper.st_gid, ID_HOST + 7);
> +
> +       EXPECT_EQ(close(ovl), 0);
> +}
> +
>  TEST_HARNESS_MAIN
> --
> 2.47.3
>

If Christian says this is a legit setup you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Otherwise, we do not need two tests and we do not need any opt-in mount
options, not index and not nfs_export, the mount should just fail.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers
  2026-09-21  8:03 ` [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Amir Goldstein
@ 2026-09-21  8:22   ` Jérémy Jean
  0 siblings, 0 replies; 9+ messages in thread
From: Jérémy Jean @ 2026-09-21  8:22 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: brauner, miklos, linux-unionfs, linux-kernel

On 2026-09-21 10:03, Amir Goldstein wrote:
> On Sun, Sep 20, 2026 at 10:33 PM Jérémy Jean
> <Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>> TBH, I don't know whether this layout may actually
>> happen in production, but it looks like a weird feature.
> 
> I do not know either.
> 
> I would like to get feedback from Christian on that, because if this
> setup is not expected I'd rather fail the mount.

Hello Amir,

Thanks for the quick review of this series.
I will wait for Christian to answer before fixing your small remarks
in a v3.

Regards,
Jérémy

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-21  8:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-20 20:32 [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Jérémy Jean
2026-09-20 20:32 ` [PATCH v2 1/3] ovl: disable nfs_export for same-sb lower layers with different idmaps Jérémy Jean
2026-09-21  8:10   ` Amir Goldstein
2026-09-20 20:32 ` [PATCH v2 2/3] selftests: overlayfs: reject handles for mixed lower idmaps Jérémy Jean
2026-09-21  8:14   ` Amir Goldstein
2026-09-20 20:32 ` [PATCH v2 3/3] selftests: overlayfs: keep index copy up with " Jérémy Jean
2026-09-21  8:18   ` Amir Goldstein
2026-09-21  8:03 ` [PATCH v2 0/3] ovl: disable nfs_export for mixed-idmap same-sb lower layers Amir Goldstein
2026-09-21  8:22   ` Jérémy Jean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®