Page MenuHomeFreeBSD

Replace sys/crypto/sha2/sha2.c with lib/libmd/sha512c.c

Authored by allanjude on Oct 17 2015, 8:52 PM.
Referenced Files
Unknown Object (File)
Sat, Jan 7, 11:43 PM
Unknown Object (File)
Sat, Jan 7, 11:15 PM
Unknown Object (File)
Dec 16 2022, 11:38 AM
Unknown Object (File)
Dec 16 2022, 9:24 AM
Unknown Object (File)
Apr 22 2017, 9:49 AM
Unknown Object (File)
Apr 17 2017, 10:36 PM
Unknown Object (File)
Apr 15 2017, 3:15 PM
Unknown Object (File)
Apr 14 2017, 7:02 PM



cperciva's libmd implementation is 5-30% faster (also seen here: )

cperciva's implementation was lacking SHA-384 which I implemented
Extend sbin/md5 to create sha384(1)

In a separate commit, I will also add SHA-512/256 (required by new ZFS features)

Chase dependancies on sys/crypto/sha2/sha2.{c,h} and replace them with sha512{c.c,.h}

Will extend reviewers to include secteam@ once the first round of issues are dealt with

Test Plan

benchmark results:

The output of the new SHA-384 implementation were verified against openssl sha384 and

Diff Detail

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

Event Timeline

allanjude retitled this revision from to Replace sys/crypto/sha2/sha2.c with lib/libmd/sha512c.c.
allanjude updated this object.
allanjude edited the test plan for this revision. (Show Details)
allanjude added reviewers: jmg, cperciva.

Just a few comments.

40 ↗(On Diff #9476)

is this tab/space matching others or is this just a phab misformating issue?

559 ↗(On Diff #9476)

I don't see bde using SHA256, we should probably remove it, though maybe as a different commit?

Looks like the module also compiles it. A quick compile of the module does look like it uses it.

561 ↗(On Diff #9476)

I don't think you need zfs here yet. I'd add ZFS when you finally enable sha512/256 support.

allanjude added inline comments.
40 ↗(On Diff #9476)

i put a tab when I shouldn't have, fixed in my local copy, will update with other changes

559 ↗(On Diff #9476)

yeah, that was already there, I didn't change it. geom_eli uses both 256 and 512, should it be listed in both places?

561 ↗(On Diff #9476)

the zfs SRCS= line references both, and again, I just updated sha2.c -> sha512c.c (and changed the order to make sense). But yeah, I could remove ZFS here and readd it in the next commit.

allanjude marked an inline comment as done.
allanjude edited edge metadata.

Minor updates to address feedback from jmg@

559 ↗(On Diff #9482)

bde != eli.. They are different, so commenting that eli uses 256 and 512 is unrelated to my bde comment.

561 ↗(On Diff #9482)

Since you're adding the line fresh, it's best to keep it minimal.

559 ↗(On Diff #9482)

that should have been a separate comment.

Unrelated to geom_bde:

geom_eli uses both SHA256 and SHA512, should it be listed in this file?

allanjude edited edge metadata.

Update the diff to put sha384 prototypes in their own file as requested by cperciva

cperciva edited edge metadata.

Looks good to me.

des added a reviewer: des.
bapt added a reviewer: bapt.
delphij added a reviewer: delphij.

Approved by so, too. Sorry for the delay and thanks for working on this.

This revision is now accepted and ready to land.Dec 27 2015, 5:19 PM
This revision was automatically updated to reflect the committed changes.