[Virtio-fs] [PATCH] Virtiofs: fix null pointer deference in directIO

[Liu Bo]

On Wed, Jul 31, 2019 at 08:39:38AM -0400, Vivek Goyal wrote:
> On Tue, Jul 30, 2019 at 03:29:39PM -0700, Liu Bo wrote:
> > On Tue, Jul 30, 2019 at 11:30:41AM -0400, Vivek Goyal wrote:
> > > Hi Liu Bo,
> > > 
> > > I have committed your patch for now here.
> > > 
> > > https://github.com/rhvgoyal/linux/commits/virtio-fs-dev-5.1
> > > 
> > > I also added a patch to wait for page reference count to drop to 1 before
> > > range is reclaimed. It slows down mmap() workload very significantly
> > > though.
> > >
> > 
> > Yes, the impact on mmap is expected because of the 'unmap' in layout checking,
> > it'd be helpful to check layout on the dmap range instead of the whole inode.
> > 
> > However, there is a race bug in the current code between mmap workloads and dmap
> > reclaim, since the reclaim does not unmap the dmap range at all, such that the
> > dmap range might still stay in some process's page table, if such a dmap is
> > reclaimed, the situation would be worse.  So with unmapping the whole inode in
> > layout checking, at least it is correct in term of that race.
> 
> Can you elaborate more on this. I am not sure I am able to see the race
> you are talking about. Assume we implemented logic to unmap only pages
> which are in dmap (and not whole inode).
>

Sorry for confusing you, what I was trying to say is that the above
problem exists without doing unmapping pages (no matter within the
whole inode or only the dmap range), IOW, before commit "fuse: Free
dax mapping range only if page refcount is 1", the race bug exists.

thanks,
-liubo

> dmap is something internal to virtio-fs. If we unmap all pages which are
> in a dmap, then dmap is free to be reclaimed. We are holding fi->i_mmap_sem,
> and that should make sure that unmapped pages don't get mapped again (page
> fault code will block on fi->i_mmap_sem).
> 
> Thanks
> Vivek