Page MenuHomeFreeBSD

Shift some of the EKCD funcs around to keep them grouped together.

Authored by markj on Jul 15 2017, 12:43 AM.
Referenced Files
Unknown Object (File)
Sun, Jan 15, 8:26 AM
Unknown Object (File)
Dec 18 2022, 6:29 AM
Unknown Object (File)
Nov 27 2022, 2:51 AM



Now that all of the kerneldumpcrypto functions are called only in
kern_shutdown.c, this is more reasonable. This also lets us reduce the
number of ifdefs.

Also Remove the keysize parameter from mkdumpheader() and instead use
the configured dumper to get the key size. For textdumps we already
"hide" the configured crypto parameters by temporarily unsetting

Test Plan

minidumps, encrypted minidumps, textdumps, all on amd64

Diff Detail

rS FreeBSD src repository - subversion
Lint Not Applicable
Tests Not Applicable

Event Timeline

markj added reviewers: cem, asomers, def, rgrimes.
cem added inline comments.
1217 ↗(On Diff #30805)

Maybe size_t or uint64_t (to match dumpsize) instead?

1280 ↗(On Diff #30805)

(Implicit zero-initialization of dumpkeysize in the non-EKCD case via bzero above.)

This revision is now accepted and ready to land.Jul 15 2017, 1:08 AM
1269 ↗(On Diff #30805)

dumpkeysize was passed to mkdumpheader() not to use the global dumper variable indirectly from MD code. I liked that we had only two functions where the dumper variable was used set_dumper() and doadump() which require some global state. The second one passes the dumper to MD code which calls dump API passing it as an argument.

What do you think about introducing dump_init_header() function which would have di as an argument so we could extract a kernel dump key size and a block size from it? Note that it doesn't make sense to create a kernel dump header and write it to a dump device which has a different block size from the one included in the header. Currently mkdumpheader() allows to do so and savecore(8) gets a dump device block size using ioctl(2), not from a kernel dump header. This issue existed before but we could fix it too meanwhile we're here.

(Oops, I forgot to submit my earlier comments in response to Conrad.)

1217 ↗(On Diff #30805)

I chose this to match the return type of kerneldumpcrypto_dumpkeysize(). I don't see any problem with keeping it a uint32_t?

1269 ↗(On Diff #30805)

Yup, that sounds reasonable to me. To be clear, you're talking about renaming mkdumpheader() and having it take di as an argument, rather than having both dump_init_header() and mkdumpheader()?

1280 ↗(On Diff #30805)

Yeah, I'll add an explicit initialization instead.

1217 ↗(On Diff #30805)

There's no problem with it staying uint32_t.

1269 ↗(On Diff #30805)

Yes, I'd like to rename mkdumpheader() to dump_init_header(), provide a const di pointer as the first argument and remove blksz from the arguments list.

markj edited edge metadata.
markj marked 2 inline comments as done.
  • Rename mkdumpheader() to dump_init_header(), and have it take the dumperinfo as a parameter.
This revision now requires review to proceed.Aug 4 2017, 11:46 PM
This revision is now accepted and ready to land.Aug 6 2017, 3:51 PM
This revision was automatically updated to reflect the committed changes.