On Fri, Jun 9, 2023 at 5:28 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Fri, Jun 9, 2023 at 4:15 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > > On Fri, 9 Jun 2023 at 09:32, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > Miklos, > > > > > > This is the solution that we discussed for removing FMODE_NONOTIFY > > > from overlayfs real files. > > > > > > My branch [1] has an extra patch for remove FMODE_NONOTIFY, but > > > I am still testing the ovl-fsnotify interaction, so we can defer > > > that step to later. > > > > > > I wanted to post this series earlier to give more time for fsdevel > > > feedback and if these patches get your blessing and the blessing of > > > vfs maintainers, it is probably better that they will go through the > > > vfs tree. > > > > > > I've tested that overlay "fake" path are still shown in /proc/self/maps > > > and in the /proc/self/exe and /proc/self/map_files/ symlinks. > > > > > > The audit and tomoyo use of file_fake_path() is not tested > > > (CC maintainers), but they both look like user displayed paths, > > > so I assumed they's want to preserve the existing behavior > > > (i.e. displaying the fake overlayfs path). > > > > I did an audit of all ->vm_file and found a couple of missing ones: > > Wait, but why only ->vm_file? > We were under the assumption the fake path is only needed > for mapped files, but the list below suggests that it matters > to other file operations as well... > > > > > dump_common_audit_data > > ima_file_mprotect > > common_file_perm (I don't understand the code enough to know whether > > it needs fake dentry or not) > > aa_file_perm > > __file_path_perm > > print_bad_pte > > file_path > > seq_print_user_ip > > __mnt_want_write_file > > __mnt_drop_write_file > > file_dentry_name > > > > Didn't go into drivers/ and didn't follow indirect calls (e.g. > > f_op->fsysnc). I also may have missed something along the way, but my > > guess is that I did catch most cases. > > Wow. So much for 3-4 special cases... > > Confused by some of the above. > > Why would we want __mnt_want_write_file on the fake path? > We'd already taken __mnt_want_write on overlay and with > real file we need __mnt_want_write on the real path. > > For IMA/LSMs, I'd imagine that like fanotify, they would rather get > the real path where the real policy is stored. > If some log files end with relative path instead of full fake path > it's probably not the worst outcome. > > Thoughts? Considering the results of your audit, I think I prefer to keep f_path fake and store real_path in struct file_fake for code that wants the real path. This will keep all logic unchanged, which is better for my health. and only fsnotify (for now) will start using f_real_path() to generate events on real fs objects. Thanks, Amir.
