Linux: zfs_fillpage() should handle partial pages from end of file

[ryao]

Motivation and Context

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.

Description

We modify zfs_fillpage() to pass io_len to dmu_read() and then zero the remainder of the page whenever io_len is less than a page.

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.

How Has This Been Tested?

Clang's static analyzer no longer reports the issue with this patch applied. The buildbot can test it.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

This was a good find!

[ryao]

I described an idea for a tool for catching bugs like this to the LLVM discourse:

https://discourse.llvm.org/t/is-there-any-way-to-identify-potential-execution-paths-that-call-panic-from-public-functions/68771

The idea is to do something similar to taint analysis, where we will designate ZPL and zvol functions as sources and zfs_panic_recovery() and VERIFY*() statements as sinks. If any invocation of the source functions can result in execution of the sink functions, then the tool should report the execution path and conditions, since that is likely a bug. This differs from taint analysis because taint analysis only reports when tainted data is passed to a sink function and what we want is to know when a sink function can be executed as a result of a source function.

This hypothetical tool should be powerful enough to catch the issue reported here without help from 89cd219 unlike clang's static analyzer. It would be limited by its inability to look across thread stacks such as in the case of zio_wait(), but it would still be a nice addition to our existing tools.

[ryao]

I have rebased this on master and repushed.