On Mon, Aug 05, 2024 at 02:13:49PM +0200, Jan Kara wrote:
> On Fri 02-08-24 12:03:57, Josef Bacik wrote:
> > On Thu, Aug 01, 2024 at 11:40:25PM +0200, Jan Kara wrote:
> > > On Thu 25-07-24 14:19:47, Josef Bacik wrote:
> > > > FS_PRE_ACCESS or FS_PRE_MODIFY will be generated on page fault depending
> > > > on the faulting method.
> > > >
> > > > This pre-content event is meant to be used by hierarchical storage
> > > > managers that want to fill in the file content on first read access.
> > > >
> > > > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > > ...
> > > > @@ -3287,6 +3288,35 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
> > > > if (unlikely(index >= max_idx))
> > > > return VM_FAULT_SIGBUS;
> > > >
> > > > + /*
> > > > + * If we have pre-content watchers then we need to generate events on
> > > > + * page fault so that we can populate any data before the fault.
> > > > + *
> > > > + * We only do this on the first pass through, otherwise the populating
> > > > + * application could potentially deadlock on the mmap lock if it tries
> > > > + * to populate it with mmap.
> > > > + */
> > > > + if (fault_flag_allow_retry_first(vmf->flags) &&
> > > > + fsnotify_file_has_content_watches(file)) {
> > >
> > > I'm somewhat nervous that if ALLOW_RETRY isn't set, we'd silently jump into
> > > readpage code without ever sending pre-content event and thus we'd possibly
> > > expose invalid content to userspace? I think we should fail the fault if
> > > fsnotify_file_has_content_watches(file) && !(vmf->flags &
> > > FAULT_FLAG_ALLOW_RETRY).
> >
> > I was worried about this too but it seems to not be the case that we'll not ever
> > have ALLOW_RETRY. That being said I'm fine turning this into a sigbus.
>
> Do you mean that with your workloads we always have ALLOW_RETRY set? As I
> wrote, currently you'd have to try really hard to hit such paths but they
> are there - for example if you place uprobe on an address in a VMA that is
> not present, the page fault is going to happen without ALLOW_RETRY set.
