Is zfs(4) the right reference? I don't see anything in there about holes.

Need to indicate that 12.x has a variant of the faulty dirtiness check, but it may not be possible to encounter the issue there (in particular, copy_file_range does not exist and 12.x ZFS does not support SEEK_HOLE as far as I can tell).

Can update this with some notes from @mav's text in PR275308 - in particular mentioning the incomplete dirtiness check.

I included the zfs(4) reference for the description of the sysctl -- i.e.,

zfs_dmu_offset_next_sync=1|0 (int)
        Enable forcing TXG sync to find holes.  When enabled forces ZFS
        to sync data when SEEK_HOLE or SEEK_DATA flags are used allowing
        holes in a file to be accurately reported.  When disabled holes
        will not be reported in recently dirtied files.

I can make this "See the zfs(4) man page for more information on the effect of setting the sysctl to zero." or something like that

I do see SEEK_HOLE support in stable/12 ZFS. But you are right about copy_file_range().

I'd say: "hole may incorrectly be reported where data were written". And "The data may instead be interpreted as zero bytes." -- explicit reads are not affected, apps just skip them.

Again, not while reading.

Perhaps add:

An OpenZFS dnode is a data structure used to represent and manage metadata about files and directories.

In file systems, "dirty" refers to data or metadata that has been modified in memory but not yet written to the storage device.

A check did not test both the dnode itself and its data for dirtiness. This provides a very small window while a file is being modified where the dirtiness check falsely reports that the dnode is clean. If this happens a hole can then reported in place of the expected data.

likelihood

applied locally

This URL is redirected to https://docs.freebsd.org/en/books/handbook/kernelconfig/ , should we update it (and the template)?

I think so yes. The templates are in the doc tree if you want to update.

"uncommitted" might confuse people who aren't familiar with filesystem/database jargon?

possibly, although it is probably understandable with the context of the previous sentence. I can't think of a good way to rewrite it without basically repeating the previous sentence.

This might just be a function of it's good to be complete when describing the problem/impact, but this first sentence repeats the same information that the last sentence of the previous section has. I guess it's fine, especially if you want to just jump to the impact section, but it's weird when reading the whole thing.

Otherwise, I think I can understand the problem and impact, and the remediation steps are also clear.

I like the additional bits added here.

@rob.norris_klarasystems.com just mentioned that it's been reproduced on Illumos https://www.illumos.org/issues/16087 - so we probably want to update to "Affects: All supported versions of FreeBSD" but leave a note that a patch for FreeBSD 12.4 is not yet available but the advisory will be available when it is.

This can be read as if the impact is only during a file copy (as a non-native speaker I could understand "If this occurs.." as "I'm doing a file copy and if I'm unlucky, the issue is triggered, and corruption of the target happens"), but the file copy is just one use case. Should we make it more explicit that the we make a special case here for a file copy?