linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] overlayfs update for 6.16
@ 2025-06-05 14:51 Miklos Szeredi
  2025-06-05 19:34 ` Linus Torvalds
  0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2025-06-05 14:51 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: overlayfs, linux-fsdevel, linux-kernel

Hi Linus,

Please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/overlayfs/vfs.git
tags/ovl-update-6.16

- Fix a regression in getting the path of an open file (e.g.  in
/proc/PID/maps) for a nested overlayfs setup  (André Almeida)

- The above fix contains a cast to non-const, which is not actually
needed.  So add the necessary helpers postfixed with _c that allow the
cast to be removed (touches vfs files but only in trivial ways)

- Support data-only layers and verity in a user namespace
(unprivileged composefs use case)

- Fix a gcc warning (Kees)

- Cleanups

Thanks,
Miklos

---
André Almeida (1):
      ovl: Fix nested backing file paths

Kees Cook (1):
      ovl: Check for NULL d_inode() in ovl_dentry_upper()

Miklos Szeredi (4):
      ovl: make redirect/metacopy rejection consistent
      ovl: relax redirect/metacopy requirements for lower -> data redirect
      ovl: don't require "metacopy=on" for "verity"
      vfs: change 'struct file *' argument to 'const struct file *'
where possible

Thorsten Blum (4):
      ovl: Use str_on_off() helper in ovl_show_options()
      ovl: Replace offsetof() with struct_size() in ovl_cache_entry_new()
      ovl: Replace offsetof() with struct_size() in ovl_stack_free()
      ovl: Annotate struct ovl_entry with __counted_by()

---
 Documentation/filesystems/overlayfs.rst |  7 +++
 fs/file_table.c                         | 10 ++--
 fs/internal.h                           |  1 +
 fs/overlayfs/file.c                     |  4 +-
 fs/overlayfs/namei.c                    | 98 ++++++++++++++++++++-------------
 fs/overlayfs/ovl_entry.h                |  2 +-
 fs/overlayfs/params.c                   | 40 ++------------
 fs/overlayfs/readdir.c                  |  4 +-
 fs/overlayfs/util.c                     |  9 ++-
 include/linux/fs.h                      | 12 ++--
 10 files changed, 97 insertions(+), 90 deletions(-)

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

* Re: [GIT PULL] overlayfs update for 6.16
  2025-06-05 14:51 [GIT PULL] overlayfs update for 6.16 Miklos Szeredi
@ 2025-06-05 19:34 ` Linus Torvalds
  2025-06-06  6:17   ` Amir Goldstein
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2025-06-05 19:34 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: overlayfs, linux-fsdevel, linux-kernel

On Thu, 5 Jun 2025 at 07:51, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> - The above fix contains a cast to non-const, which is not actually
> needed.  So add the necessary helpers postfixed with _c that allow the
> cast to be removed (touches vfs files but only in trivial ways)

Grr.

I despise those "trivial ways".

In particular, I despise how this renames the backing_file_user_path()
helper to something actively *worse*.

The "_c()" makes no sense as a name. Yes, I realize that the "c"
stands for "const", but it still makes absolutely zero sense, since
everybody wants the const version.

The only user of the non-const version is the *ointernal*
implementation that is never exported to other modules, and that could
have the special name.

Although I suspect it doesn't even need it, it could just use the
backing_file(f) macro directly and that should just be moved to
internal.h, and then the 'const'ness would come from the argument as
required.

In fact, most of the _internal_ vfs users don't even want the
non-const version, ie as far as I can tell the user in
file_get_write_access() would be perfectly happy with the const
version too.

So you made the *normal* case have an odd name, and then kept the old
sane name for the case nobody else really wants to see.

If anything, the internal non-const version is the one that should be
renamed (and *not* using some crazy "_nc()" postfix nasty crud). Not
the one that gets exported and that everybody wants.

So I could fix up that last commit to not hate it, but honestly, I
don't want that broken state in the kernel in the first place.

Please do that thing properly. Not this hacky and bass-ackwards way.

             Linus

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

* Re: [GIT PULL] overlayfs update for 6.16
  2025-06-05 19:34 ` Linus Torvalds
@ 2025-06-06  6:17   ` Amir Goldstein
  2025-06-06  6:36     ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2025-06-06  6:17 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]

On Thu, Jun 5, 2025 at 9:34 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, 5 Jun 2025 at 07:51, Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > - The above fix contains a cast to non-const, which is not actually
> > needed.  So add the necessary helpers postfixed with _c that allow the
> > cast to be removed (touches vfs files but only in trivial ways)
>

I must have snoozed the review of this one :-/

> Grr.
>
> I despise those "trivial ways".
>
> In particular, I despise how this renames the backing_file_user_path()
> helper to something actively *worse*.
>
> The "_c()" makes no sense as a name. Yes, I realize that the "c"
> stands for "const", but it still makes absolutely zero sense, since
> everybody wants the const version.
>
> The only user of the non-const version is the *ointernal*
> implementation that is never exported to other modules, and that could
> have the special name.
>
> Although I suspect it doesn't even need it, it could just use the
> backing_file(f) macro directly and that should just be moved to
> internal.h, and then the 'const'ness would come from the argument as
> required.
>
> In fact, most of the _internal_ vfs users don't even want the
> non-const version, ie as far as I can tell the user in
> file_get_write_access() would be perfectly happy with the const
> version too.
>
> So you made the *normal* case have an odd name, and then kept the old
> sane name for the case nobody else really wants to see.
>
> If anything, the internal non-const version is the one that should be
> renamed (and *not* using some crazy "_nc()" postfix nasty crud). Not
> the one that gets exported and that everybody wants.
>

IMO, it would be nicer to use backing_file_set_user_path()
(patch attached).

> So I could fix up that last commit to not hate it, but honestly, I
> don't want that broken state in the kernel in the first place.
>

Would you consider pulling ovl-update-6.16^
and applying the attached patch [*]?

Thanks,
Amir.

[*] I did not include the removal of non-const casting to keep this
patch independent of the ovl PR.
Feel free to add it to my patch or I can send the patch post merge
or cleanup of casting post merge.

[-- Attachment #2: fs-constify-file-ptr-in-backing_file_user_path.patch --]
[-- Type: text/x-patch, Size: 3262 bytes --]

From bc79491bf671b8a1d6603e0367963341339f943f Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 6 Jun 2025 07:46:02 +0200
Subject: [PATCH] fs: constify file ptr in backing_file_user_path()

Add internal helper backing_file_set_user_path() for the only
two cases that need to modify user path.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/backing-file.c  |  4 ++--
 fs/file_table.c    | 13 ++++++++-----
 fs/internal.h      |  1 +
 include/linux/fs.h |  2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/backing-file.c b/fs/backing-file.c
index 763fbe9b72b2..8c7396bff121 100644
--- a/fs/backing-file.c
+++ b/fs/backing-file.c
@@ -41,7 +41,7 @@ struct file *backing_file_open(const struct path *user_path, int flags,
 		return f;
 
 	path_get(user_path);
-	*backing_file_user_path(f) = *user_path;
+	backing_file_set_user_path(f, user_path);
 	error = vfs_open(real_path, f);
 	if (error) {
 		fput(f);
@@ -65,7 +65,7 @@ struct file *backing_tmpfile_open(const struct path *user_path, int flags,
 		return f;
 
 	path_get(user_path);
-	*backing_file_user_path(f) = *user_path;
+	backing_file_set_user_path(f, user_path);
 	error = vfs_tmpfile(real_idmap, real_parentpath, f, mode);
 	if (error) {
 		fput(f);
diff --git a/fs/file_table.c b/fs/file_table.c
index c04ed94cdc4b..8ac2fbbd4f6d 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -52,17 +52,20 @@ struct backing_file {
 	};
 };
 
-static inline struct backing_file *backing_file(struct file *f)
-{
-	return container_of(f, struct backing_file, file);
-}
+#define backing_file(f) container_of(f, struct backing_file, file)
 
-struct path *backing_file_user_path(struct file *f)
+struct path *backing_file_user_path(const struct file *f)
 {
 	return &backing_file(f)->user_path;
 }
 EXPORT_SYMBOL_GPL(backing_file_user_path);
 
+void backing_file_set_user_path(struct file *f, const struct path *path)
+{
+	backing_file(f)->user_path = *path;
+}
+EXPORT_SYMBOL_GPL(backing_file_set_user_path);
+
 static inline void file_free(struct file *f)
 {
 	security_file_free(f);
diff --git a/fs/internal.h b/fs/internal.h
index 213bf3226213..3860b022e57c 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -101,6 +101,7 @@ extern void chroot_fs_refs(const struct path *, const struct path *);
 struct file *alloc_empty_file(int flags, const struct cred *cred);
 struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred);
 struct file *alloc_empty_backing_file(int flags, const struct cred *cred);
+void backing_file_set_user_path(struct file *f, const struct path *path);
 
 static inline void file_put_write_access(struct file *file)
 {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a4fd649e2c3f..c745aee9c88a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2813,7 +2813,7 @@ struct file *dentry_open_nonotify(const struct path *path, int flags,
 				  const struct cred *cred);
 struct file *dentry_create(const struct path *path, int flags, umode_t mode,
 			   const struct cred *cred);
-struct path *backing_file_user_path(struct file *f);
+struct path *backing_file_user_path(const struct file *f);
 
 /*
  * When mmapping a file on a stackable filesystem (e.g., overlayfs), the file
-- 
2.34.1


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

* Re: [GIT PULL] overlayfs update for 6.16
  2025-06-06  6:17   ` Amir Goldstein
@ 2025-06-06  6:36     ` Miklos Szeredi
  2025-06-06  9:33       ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2025-06-06  6:36 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Linus Torvalds, overlayfs, linux-fsdevel, linux-kernel

On Fri, 6 Jun 2025 at 08:17, Amir Goldstein <amir73il@gmail.com> wrote:

> IMO, it would be nicer to use backing_file_set_user_path()
> (patch attached).

Looks nice, thanks.

> Would you consider pulling ovl-update-6.16^
> and applying the attached patch [*]?
>
> Thanks,
> Amir.
>
> [*] I did not include the removal of non-const casting to keep this
> patch independent of the ovl PR.
> Feel free to add it to my patch or I can send the patch post merge
> or cleanup of casting post merge.

I'll redo the PR with your patch.

Thanks,
Miklos

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

* Re: [GIT PULL] overlayfs update for 6.16
  2025-06-06  6:36     ` Miklos Szeredi
@ 2025-06-06  9:33       ` Miklos Szeredi
  2025-06-07 10:41         ` Amir Goldstein
  0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2025-06-06  9:33 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Linus Torvalds, overlayfs, linux-fsdevel, linux-kernel

On Fri, 6 Jun 2025 at 08:36, Miklos Szeredi <miklos@szeredi.hu> wrote:

> I'll redo the PR with your patch.

Pushed to #overlayfs-next.

I'll drop this from the PR, since it's just a cleanup.  It still won't
break anything (and that's what I meant by "trivial"), but it can wait
a cycle or at least a few rc's.

Thanks,
Miklos

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

* Re: [GIT PULL] overlayfs update for 6.16
  2025-06-06  9:33       ` Miklos Szeredi
@ 2025-06-07 10:41         ` Amir Goldstein
  0 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2025-06-07 10:41 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Linus Torvalds, overlayfs, linux-fsdevel, linux-kernel

On Fri, Jun 6, 2025 at 11:33 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, 6 Jun 2025 at 08:36, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > I'll redo the PR with your patch.
>
> Pushed to #overlayfs-next.
>
> I'll drop this from the PR, since it's just a cleanup.  It still won't
> break anything (and that's what I meant by "trivial"), but it can wait
> a cycle or at least a few rc's.
>

I'll send the cleanup patch to Christian.
It's best if it goes in via the vfs tree anyway.

Thanks,
Amir.

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

end of thread, other threads:[~2025-06-07 10:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-05 14:51 [GIT PULL] overlayfs update for 6.16 Miklos Szeredi
2025-06-05 19:34 ` Linus Torvalds
2025-06-06  6:17   ` Amir Goldstein
2025-06-06  6:36     ` Miklos Szeredi
2025-06-06  9:33       ` Miklos Szeredi
2025-06-07 10:41         ` Amir Goldstein

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).