Page MenuHomeFreeBSD

sendfile: Return the object size from sendfile_getobj()

Authored by markj on Feb 19 2021, 9:46 PM.
Referenced Files
Unknown Object (File)
Mar 4 2023, 9:25 PM
Unknown Object (File)
Feb 15 2023, 6:25 AM
Unknown Object (File)
Feb 6 2023, 10:58 AM
Unknown Object (File)
Jan 15 2023, 4:30 AM
Unknown Object (File)
Dec 30 2022, 5:22 AM
Unknown Object (File)
Dec 16 2022, 5:03 AM
Unknown Object (File)
Nov 28 2022, 11:30 AM



I believe this is more correct, since subsequent bounds checking, e.g.
by vm_pager_has_page(), is done using the object size. It also fixes an
unlocked read of the object size when a shared memory object is used.
I also changed to using the object read lock instead of the write lock,
as I cannot see a reason for the latter.

As a side effect, this change mitigates a problem observed on ZFS when
transmitting a file with sendfile() at the same time that the file is
being extended by a writer. The basic problem is that the file size (as
returned by VOP_GETATTR()) and the object size (updated by
vnode_pager_setsize()) may be out of sync unless the exclusive vnode
lock is held. In particular, there may be a window where the file size
is larger than the object size. So, if sendfile() is transmitting the
whole file, it will transmit <file size> bytes, but vm_pager_has_page()
may return false during this window. In this case, sendfile() will
zero-fill the page, incorrectly assuming that there is a hole in the
file. This can manifest as apparent data corruption if the file is read
back from the page cache.

Following a suggestion from kib I also started looking at using a
rangelock to ensure that the length of the file does not change as we
are paging in. (Use of async getpages means that the file contents
cannot be kept consistent, I believe.) However, I would like to have a
simpler mitigation for 13.0, so I propose this change first.

Test Plan

@dumbbell reported the problem and provided a test case to
demonstrate it. I would ask Peter to test it before committing.

Diff Detail

rG FreeBSD src repository
Lint Not Applicable
Tests Not Applicable

Event Timeline

markj requested review of this revision.Feb 19 2021, 9:46 PM
markj added reviewers: glebius, kib, chs.
markj added a subscriber: dumbbell.
This revision is now accepted and ready to land.Feb 20 2021, 12:51 PM


I tested this patch with the testsuite at work which failed because of the issue and it works perfectly now.

Thank you very much!

  • Fixes for tmpfs and shm objects.

Tested by: pho

This revision now requires review to proceed.Feb 24 2021, 3:23 PM

Not calling VOP_GETATTR was a feature of the patch. At least, you can do it based on the object type?

This revision is now accepted and ready to land.Feb 24 2021, 6:39 PM
  • Avoid VOP_GETATTR when the object type is OBJT_VNODE.
  • Add a short comment explaining why we prefer the pager size.
This revision now requires review to proceed.Feb 24 2021, 7:13 PM
This revision is now accepted and ready to land.Feb 24 2021, 7:33 PM