Fix kernel panic induced by redacted send

[pcd1193182]

Signed-off-by: Paul Dagnelie pcd@delphix.com

Motivation and Context

There is a bug in the binary search in the redacted send resume logic. This changes simplifies the code by switching to a per-record binary search, and fixes outstanding issues causing a kernel panic.

How Has This Been Tested?

Passes the zfs-test suite and testing involving redacted send/recv

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.

[ahrens]

For folks who may run into this bug in the future (in older releases), it would be helpful if the PR/commit message contained a description of the bugs that this is fixing, and their impact. I think it's:

1. maxbufid can be decremented to -1, causing us to read the last possible block of the object (offset 2^64 - 128K to offset 2^64-1) instead of the one we wanted. maxbufid = midbufid - 1; Not sure what the impact is upstream?
2. when examining non-last blocks, we compare with an entry in the middle of the block, rather than the last entry in the block. redact_block_zb_compare(&buf[last_entry(rl, bufsize, maxbufid)], resume) maxbufid should be midbufid. This can cause us to not invoke the callback for some of the redaction records, when resuming.

[behlendorf]

It looks like this change causes an ASSERT to be hit when running the redacted_send tests.

[12368.647136] VERIFY3(redact_block_zb_compare(rb, resume) >= 0) failed (-1 >= 0)
[12368.649085] PANIC at dsl_bookmark.c:1684:dsl_redaction_list_traverse()
[12368.650696] Showing stack for process 3370004
[12368.651814] CPU: 1 PID: 3370004 Comm: redact_list Kdump: loaded Tainted: P           OE    --------- -  - 4.18.0-240.1.1.el8_3.x86_64 #1
[12368.654914] Hardware name: Amazon EC2 m5d.large/, BIOS 1.0 10/16/2017
[12368.656504] Call Trace:
[12368.657137]  dump_stack+0x5c/0x80
[12368.657979]  spl_panic+0xd3/0xfb [spl]
[12368.669539]  dsl_redaction_list_traverse+0x321/0x350 [zfs]
[12368.672040]  redact_list_thread+0x7f/0xe0 [zfs]
[12368.674333]  thread_generic_wrapper+0x78/0xb0 [spl]
[12368.675547]  kthread+0x112/0x130

[pcd1193182]

The problem is that the assertion is incorrect; we won't necessarily always come out of the binary search after or equal to the record that overlaps with the resume point. In particular, there may be no such record; if we are beyond the final redaction record, for example, the binary search will return the final redaction record, but the resume point will be after it. There may be other situations where this can occur, depending on exactly how the binary search behaves. With that in mind, I've reverted that portion of the change, and am testing it now.