On 2026-06-26 17:18 +0200, Amir Goldstein wrote:
> On Thu, Jun 25, 2026 at 12:36 PM Christian Brauner <[email protected]> wrote:
> >
> > On 2026-06-23 17:34:47+02:00, Amir Goldstein wrote:
> > > On Tue, Jun 23, 2026 at 4:55 PM Pavel Tikhomirov
> > > <[email protected]> wrote:
> > >
> > > > On 6/23/26 15:48, Johannes Weiner wrote:
> > > >
> > > > Yes, AFAIU in overlay when we use realfile we should always use it
> > > > with_ovl_creds(), even though I don't think there is anything cred
> > > > related
> > > > in filemap_cachestat(), I still think we should follow the common
> > > > pattern
> > > > other overlay helpers use (similar to ovl_fadvise() and ovl_flush()).
> > > >
> > > > note: Actually some places get ovl_real_file() and use it without
> > > > with_ovl_creds(), e.g.: ovl_read_iter, ovl_write_iter, ovl_splice_read,
> > > > ovl_splice_write. But those look more of an exception than the general
> > > > rule. All other instances use with_ovl_creds().
> > >
> > > Use with_ovl_creds() is a good practice to keep the mental security model,
> > > but it is useless if the security check (can_do_cachestat) is not in the
> > > vfs helper (vfs_cachestat), so please move it there.
> > >
> > > Also it kind of makes more sense to check (flags != 0) in sys_cachestats
> > > before checking permissions.
> > >
> > > > Also there are simingly no other file_operations which return "realfile"
> > > > for further processing, mostly the operation from fsops simply replaces
> > > > general operation with its own logic completely.
> > > >
> > > > Thanks for your review!
> > > >
> > > > ps: Hope overlay maintainers will correctly if I'm getting this wrong.
> > >
> > > I don't think this is wrong per-se, except for can_do_cachestat().
> > >
> > > Just be aware that the real file could change from one cachestat
> > > call to the next (i.e. due to copy up).
> >
> > I'm really grump about adding a new file operation just for a
> > special-sauce system call which is under a CONFIG_* option even. We're
> > not going to set the precedent of piling on custom file operations for a
> > single filesystem everytime someone adds a new system call unless
> > absolutely necessary.
>
> I had a similar reaction.
>
> > This looks like it could just use a new helper in
> > fs/backing_file.c that the cachestat thing can call to use the correct
> > file.
> >
>
> I was considering suggesting this as well.
> Having f_real_file() complement
>
> but technically, can_do_cachestat() should be checked against
> the overlayfs file/inode AND also against the real file/inode with
> ovl_creds.
>
> I'd love to be able to provide a backing_file "template" for
> operations, but I don't have a good idea how to do that.
> Do you?
diff --git a/fs/file_table.c b/fs/file_table.c
index c68b8c0a4097..d93059d53e88 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -57,6 +57,8 @@ struct backing_file {
#ifdef CONFIG_SECURITY
void *security;
#endif
+ unsigned int flags;
+ struct cred *backing_cred;
};
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 27cc07738f33..64e07ebc43b0 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -50,6 +50,12 @@ static struct file *ovl_open_realfile(const struct file
*file,
realfile = backing_file_open(file,
flags, realpath,
current_cred());
+
+ // Raises ->flags |= BACKING_FILE_CREDS and sets
->creds to ->backing_cred.
+ backing_file_set_creds(file,
ovl_creds(file_inode(file)->i_sb));
+ // Now add a helper that performs permission checks on
+ // both creds for that cachestat() thing and returns
+ // the actual file to operate on.
}
}
something like that?
