netdump: send key before dump, in case dump fails

[vangyzen]

Summary

Previously, if an encrypted netdump failed, such as due to a timeout or
network failure, the key was not saved, so a partial dump was
completely useless.

Send the key first, so the partial dump can be decrypted, because even a
partial dump can be useful.

Test Plan

I ran ls in a loop during a netdump and watched netdumpd create the
key file first. Decryption was successful.

[markj]

sys/kern/kern_shutdown.c

1504

How can this situation arise? keysize will be 0 if kdc is NULL.

sys/netinet/netdump/netdump_client.c

352

Do you need a netdump_cleanup() call in the error paths?

[bdrewery]

sys/netinet/netdump/netdump_client.c

352

Yup, since line 339 is setting the global pcb.

[vangyzen]

• CR feedback: call cleanup on error paths

[vangyzen]

sys/kern/kern_shutdown.c

1504

This avoids a NULL dereference. In the old code, we dereferenced kdc without testing for NULL. Unfortunately, I don't have more detail, since I made this change locally six months ago, failed to explain this bit in the internal code review, and forgot to upstream it right away. It _seems_ like we should hit it easily. Maybe clang is reordering things enough to accidentally protect us?

sys/netinet/netdump/netdump_client.c

352

Good catch. Thanks.

[vangyzen]

sys/kern/kern_shutdown.c

1504

Or maybe I made this change purely for symmetry with dump_start, and the NULL dereference would have happened in that function without this extra guard?

[markj]

sys/kern/kern_shutdown.c

1504

I see. kdc->kdc_dumpkey will just evaluate to (char *)kdc + offsetof(struct kerneldumpcrypto, kdc_dumpkey) because kdc_dumpkey is a VLA, so no dereference happens. Do you have some local changes which invalidate that?

[vangyzen]

sys/kern/kern_shutdown.c

1504

I see. Thanks. I obviously didn't look that closely at the type. My bad.

As it turns out, key is no longer used in this function, so I'll just remove it.

[vangyzen]

• CR feedback: remove unused "key" local var