There's a race in fuse's readdir cache that can result in an uninitilized
page being read. The page lock is supposed to prevent this from happening
but in the following case it doesn't:
Two fuse_add_dirent_to_cache() start out and get the same parameters
(size=0,offset=0). One of them wins the race to create and lock the page,
after which it fills in data, sets rdc.size and unlocks the page.
In the meantime the page gets evicted from the cache before the other
instance gets to run. So that one also creates the page, but finds the
size to be mismatched, bails out and leaves the uninitialized page in the
cache.
Fix by making sure that this spurious creation gets undone, so that
the cache always contains valid data (while the page lock is held). The
__GFP_ZERO is to differentiate this from the case when the page wasn't
evicted between the racing additions, as well as a nice cleanup.
Reported-by: Frank Sorenson <fsorenso@xxxxxxxxxx>
Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
---
Willy,
delete_from_page_cache() exporting was just removed, and seems like it's
deprecated in favor of the folio functions. Should this code use the folio
one and export that? Is the code even correct to begin with?
Thanks,
Miklos
fs/fuse/readdir.c | 18 ++++++++++--------
mm/folio-compat.c | 1 +
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index b4e565711045..4284e28be2e8 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -65,25 +65,27 @@ static void fuse_add_dirent_to_cache(struct file *file,
page = find_lock_page(file->f_mapping, index);
} else {
page = find_or_create_page(file->f_mapping, index,
- mapping_gfp_mask(file->f_mapping));
+ mapping_gfp_mask(file->f_mapping) | __GFP_ZERO);
}
if (!page)
return;
spin_lock(&fi->rdc.lock);
+ addr = kmap_local_page(page);
/* Raced with another readdir */
if (fi->rdc.version != version || fi->rdc.size != size ||
- WARN_ON(fi->rdc.pos != pos))
- goto unlock;
+ WARN_ON(fi->rdc.pos != pos)) {
+ /* Was this page just created? */
+ if (!offset && !((struct fuse_dirent *) addr)->namelen)
+ delete_from_page_cache(page);
+ goto unmap;
+ }
- addr = kmap_local_page(page);
- if (!offset)
- clear_page(addr);
memcpy(addr + offset, dirent, reclen);
- kunmap_local(addr);
fi->rdc.size = (index << PAGE_SHIFT) + offset + reclen;
fi->rdc.pos = dirent->off;
-unlock:
+unmap:
+ kunmap_local(addr);
spin_unlock(&fi->rdc.lock);
unlock_page(page);
put_page(page);
diff --git a/mm/folio-compat.c b/mm/folio-compat.c
index e1e23b4947d7..83aaffa6d701 100644
--- a/mm/folio-compat.c
+++ b/mm/folio-compat.c
@@ -128,6 +128,7 @@ void delete_from_page_cache(struct page *page)
{
return filemap_remove_folio(page_folio(page));
}
+EXPORT_SYMBOL(delete_from_page_cache);
int try_to_release_page(struct page *page, gfp_t gfp)
{
--
2.37.3
