Message ID | 20210622160015.18004-1-jack@suse.cz (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | dax: Fix ENOMEM handling in grab_mapping_entry() | expand |
Hi Jan, I was out for a few days... On Tue, Jun 22, 2021 at 9:00 AM Jan Kara <jack@suse.cz> wrote: > > grab_mapping_entry() has a bug in handling of ENOMEM condition. Suppose > we have a PMD entry at index I which we are downgrading to a PTE entry. > grab_mapping_entry() will set pmd_downgrade to true, lock the entry, > clear the entry in xarray, and decrement mapping->nrpages. The it will > call: > > entry = dax_make_entry(pfn_to_pfn_t(0), flags); > dax_lock_entry(xas, entry); > > which inserts new PTE entry into xarray. However this may fail > allocating the new node. We handle this by: > > if (xas_nomem(xas, mapping_gfp_mask(mapping) & ~__GFP_HIGHMEM)) > goto retry; > > however pmd_downgrade stays set to true even though 'entry' returned > from get_unlocked_entry() will be NULL now. And we will go again through > the downgrade branch. This is mostly harmless except that > mapping->nrpages is decremented again and we temporarily have invalid > entry stored in xarray. Fix the problem by setting pmd_downgrade to > false each time we lookup the entry we work with so that it matches > the entry we found. > > Fixes: b15cd800682f ("dax: Convert page fault handlers to XArray") > Signed-off-by: Jan Kara <jack@suse.cz> Looks good to me: Reviewed-by: Dan Williams <dan.j.williams@intel.com> I notice that Andrew already picked this up and I'm ok to let it percolate up to Linus through -mm.
diff --git a/fs/dax.c b/fs/dax.c index 62352cbcf0f4..da41f9363568 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -488,10 +488,11 @@ static void *grab_mapping_entry(struct xa_state *xas, struct address_space *mapping, unsigned int order) { unsigned long index = xas->xa_index; - bool pmd_downgrade = false; /* splitting PMD entry into PTE entries? */ + bool pmd_downgrade; /* splitting PMD entry into PTE entries? */ void *entry; retry: + pmd_downgrade = false; xas_lock_irq(xas); entry = get_unlocked_entry(xas, order);
grab_mapping_entry() has a bug in handling of ENOMEM condition. Suppose we have a PMD entry at index I which we are downgrading to a PTE entry. grab_mapping_entry() will set pmd_downgrade to true, lock the entry, clear the entry in xarray, and decrement mapping->nrpages. The it will call: entry = dax_make_entry(pfn_to_pfn_t(0), flags); dax_lock_entry(xas, entry); which inserts new PTE entry into xarray. However this may fail allocating the new node. We handle this by: if (xas_nomem(xas, mapping_gfp_mask(mapping) & ~__GFP_HIGHMEM)) goto retry; however pmd_downgrade stays set to true even though 'entry' returned from get_unlocked_entry() will be NULL now. And we will go again through the downgrade branch. This is mostly harmless except that mapping->nrpages is decremented again and we temporarily have invalid entry stored in xarray. Fix the problem by setting pmd_downgrade to false each time we lookup the entry we work with so that it matches the entry we found. Fixes: b15cd800682f ("dax: Convert page fault handlers to XArray") Signed-off-by: Jan Kara <jack@suse.cz> --- fs/dax.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)