On 8/29/23 20:36, Lei Huang wrote:
Our user space filesystem relies on fuse to provide POSIX interface.
In our test, a known string is written into a file and the content
is read back later to verify correct data returned. We observed wrong
data returned in read buffer in rare cases although correct data are
stored in our filesystem.
Fuse kernel module calls iov_iter_get_pages2() to get the physical
pages of the user-space read buffer passed in read(). The pages are
not pinned to avoid page migration. When page migration occurs, the
consequence are two-folds.
1) Applications do not receive correct data in read buffer.
2) fuse kernel writes data into a wrong place.
Using iov_iter_extract_pages() to pin pages fixes the issue in our
test.
Hmm, iov_iter_extract_pages does not exists for a long time and the code
in fuse_get_user_pages didn't change much. So if you are right, there
would be a long term data corruption for page migrations? And a back
port to old kernels would not be obvious?
What confuses me further is that
commit 85dd2c8ff368 does not mention migration or corruption, although
lists several other advantages for iov_iter_extract_pages. Other commits
using iov_iter_extract_pages point to fork - i.e. would your data
corruption be possibly related that?
Thanks,
Bernd
An auxiliary variable "struct page **pt_pages" is used in the patch
to prepare the 2nd parameter for iov_iter_extract_pages() since
iov_iter_get_pages2() uses a different type for the 2nd parameter.
Signed-off-by: Lei Huang <lei.huang@xxxxxxxxxxxxxxx>
---
fs/fuse/file.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index bc41152..715de3b 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -670,7 +670,7 @@ static void fuse_release_user_pages(struct fuse_args_pages *ap,
for (i = 0; i < ap->num_pages; i++) {
if (should_dirty)
set_page_dirty_lock(ap->pages[i]);
- put_page(ap->pages[i]);
+ unpin_user_page(ap->pages[i]);
}
}
@@ -1428,10 +1428,13 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
while (nbytes < *nbytesp && ap->num_pages < max_pages) {
unsigned npages;
size_t start;
- ret = iov_iter_get_pages2(ii, &ap->pages[ap->num_pages],
- *nbytesp - nbytes,
- max_pages - ap->num_pages,
- &start);
+ struct page **pt_pages;
+
+ pt_pages = &ap->pages[ap->num_pages];
+ ret = iov_iter_extract_pages(ii, &pt_pages,
+ *nbytesp - nbytes,
+ max_pages - ap->num_pages,
+ 0, &start);
if (ret < 0)
break;
