Skip to content

Commit

Permalink
Linux: zfs_fillpage() should handle partial pages from end of file
Browse files Browse the repository at this point in the history
After 89cd219 was merged, Clang's
static analyzer began complaining about a dead assignment in
`zfs_fillpage()`. Upon inspection, I noticed that the dead assignment
was because we are not using the calculated io_len that we should use to
avoid asking the DMU to read past the end of a file. This should result
in `dmu_buf_hold_array_by_dnode()` calling `zfs_panic_recover()`.

This issue predates 89cd219, but its
simplification of zfs_fillpage() eliminated the only use of the
assignment to io_len, which made Clang's static analyzer complain about
the issue.

Also, as a precaution, we add an assertion that io_offset < i_size. If
this ever fails, bad things will happen. Otherwise, we are blindly
trusting the kernel not to give us invalid offsets. We continue to
blindly trust it on non-debug kernels.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Brian Atkinson <batkinson@lanl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14534
  • Loading branch information
ryao authored Mar 1, 2023
1 parent 0f9ed58 commit dd108f5
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion module/os/linux/zfs/zfs_vnops_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -3977,12 +3977,16 @@ zfs_fillpage(struct inode *ip, struct page *pp)
u_offset_t io_off = page_offset(pp);
size_t io_len = PAGE_SIZE;

ASSERT3U(io_off, <, i_size);

if (io_off + io_len > i_size)
io_len = i_size - io_off;

void *va = kmap(pp);
int error = dmu_read(zfsvfs->z_os, ITOZ(ip)->z_id, io_off,
PAGE_SIZE, va, DMU_READ_PREFETCH);
io_len, va, DMU_READ_PREFETCH);
if (io_len != PAGE_SIZE)
memset((char *)va + io_len, 0, PAGE_SIZE - io_len);
kunmap(pp);

if (error) {
Expand Down

0 comments on commit dd108f5

Please sign in to comment.