On Wed, 22 Jan 2025 at 22:16, Ackerley Tng <ackerleytng@xxxxxxxxxx> wrote:
>
> Fuad Tabba <tabba@xxxxxxxxxx> writes:
>
> Hey Fuad, I'm still working on verifying all this but for now this is
> one issue. I think this can be fixed by checking if the folio->mapping
> is NULL. If it's NULL, then the folio has been disassociated from the
> inode, and during the dissociation (removal from filemap), the
> mappability can also either
>
> 1. Be unset so that the default mappability can be set up based on
> GUEST_MEMFD_FLAG_INIT_MAPPABLE, or
> 2. Be directly restored based on GUEST_MEMFD_FLAG_INIT_MAPPABLE
Thanks for pointing this out. I hadn't considered this case. I'll fix
in the respin.
> > <snip>
> >
> > +
> > +/*
> > + * Callback function for __folio_put(), i.e., called when all references by the
> > + * host to the folio have been dropped. This allows gmem to transition the state
> > + * of the folio to mappable by the guest, and allows the hypervisor to continue
> > + * transitioning its state to private, since the host cannot attempt to access
> > + * it anymore.
> > + */
> > +void kvm_gmem_handle_folio_put(struct folio *folio)
> > +{
> > + struct xarray *mappable_offsets;
> > + struct inode *inode;
> > + pgoff_t index;
> > + void *xval;
> > +
> > + inode = folio->mapping->host;
>
> IIUC this will be a NULL pointer dereference if the folio had been
> removed from the filemap, either through truncation or if the
> guest_memfd file got closed.
Ack.
> > + index = folio->index;
>
> And if removed from the filemap folio->index is probably invalid.
Ack and thanks again,
/fuad
> > + mappable_offsets = &kvm_gmem_private(inode)->mappable_offsets;
> > + xval = xa_mk_value(KVM_GMEM_GUEST_MAPPABLE);
> > +
> > + filemap_invalidate_lock(inode->i_mapping);
> > + __kvm_gmem_restore_pending_folio(folio);
> > + WARN_ON_ONCE(xa_err(xa_store(mappable_offsets, index, xval, GFP_KERNEL)));
> > + filemap_invalidate_unlock(inode->i_mapping);
> > +}
> > +
> > static bool gmem_is_mappable(struct inode *inode, pgoff_t pgoff)
> > {
> > struct xarray *mappable_offsets = &kvm_gmem_private(inode)->mappable_offsets;
