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