The experiment to consolidate some crypto functions, shared between zfs/geli.
AcceptedPublic

Authored by tsoome on Aug 22 2016, 10:09 PM.

Details

Summary

The goal is to reduce code duplication in loader by creating a separate
instance of crypto library, which can be linked if either zfs or geli
is needed. Incidentally it also resulted in some related changes, such as
geli + zfsboot (totally untested however). Also there is still some
confusion about _STAND and _STANDALONE defines, which probably will need
some attention. Also there seems to be some confusion about bsd.stand.mk
use cases.

The resulting zfsloader binary is only slightly reduced in size:
-r-xr-xr-x 1 root wheel 446464 Aug 19 08:46 zfsloader
-rw-r--r-- 1 root wheel 438272 Aug 23 00:30 zfsloader.b

in this sample, the zfsloader.b is built based on this work. Since only
sha256/sha512-256 code was duplicated, the result is quite good, I think.

There also seems to be some sort of issue preventing removal of unused
symbols from resulting binaries, the specific case was observed regarding
SHA* functions, eventually I just used preprocessor to exclude unused ones.

Most likely I have missed some related parts, only tested x86 build.

Test Plan

Tested with sha256/sha512 checksum set on zfs datasets.

The current best size for zfsloader with -Os is:
-rw-r--r-- 1 root wheel 380928 Aug 23 19:55 zfsloader

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 9675
Build 10115: arc lint + arc unit
tsoome retitled this revision from to The experiment to consolidate some crypto functions, shared between zfs/geli..Aug 22 2016, 10:09 PM
tsoome updated this object.
tsoome edited the test plan for this revision. (Show Details)
tsoome added subscribers: allanjude, imp.
emaste added a subscriber: emaste.Aug 23 2016, 4:15 PM
tsoome updated this revision to Diff 19589.Aug 23 2016, 4:53 PM

I hope I got most makefiles right now, bsd.stand.mk defaults to use -Os,
-Oz did break ficl (i think).
Updated to revision 304689.

tsoome edited the test plan for this revision. (Show Details)Aug 23 2016, 4:57 PM
avg added a subscriber: avg.Aug 23 2016, 5:24 PM
tsoome updated this revision to Diff 19614.Aug 24 2016, 6:40 AM
tsoome edited the test plan for this revision. (Show Details)

zfsboot has leftover libbootcrypto reference in Makefile

allanjude removed a subscriber: allanjude.
tsoome updated this revision to Diff 21122.EditedOct 6 2016, 6:21 PM

Updated to revision 306775.

resulting file sizes:
tsoome@freebsd:/usr/obj/usr/src/sys/boot % ls -l i386/zfsloader/zfsloader efi/boot1/boot1.efi efi/loader/loader.efi userboot/userboot/userboot.so
-rwxr-xr-x 1 root wheel 117248 Oct 6 21:10 efi/boot1/boot1.efi
-rwxr-xr-x 1 root wheel 414720 Oct 6 21:10 efi/loader/loader.efi
-rw-r--r-- 1 root wheel 380928 Oct 6 21:11 i386/zfsloader/zfsloader
-rwxr-xr-x 1 root wheel 376128 Oct 6 21:11 userboot/userboot/userboot.so
tsoome@freebsd:/usr/obj/usr/src/sys/boot %

tsoome updated this revision to Diff 21857.Oct 31 2016, 10:41 PM

Updated to revision 308153.

If you have time, could you refresh this, I'd like to look at it again.

share/mk/bsd.stand.mk
8

could we maybe also do CFLAGS.clang+= -Oz

tsoome updated this revision to Diff 29225.Jun 5 2017, 6:06 PM

Updated to revision 319604.

delphij accepted this revision.Jun 5 2017, 8:21 PM

Thanks for the work! (I thought this was already landed last year :)

The change looks good to me (and I appreciate it as a step forward -- duplicated copies of cryptographic code is a big headache for us) overall.

(Totally optional: I wonder if the cryptoboot library could be folded into libstand? Or is there some specific reasons we have to create a new library instead? I'm asking because doing so would avoid building it multiple times, and the license of the source are all BSD compatible.)

This revision has a positive review.Jun 5 2017, 8:21 PM
tsoome added a comment.Jun 5 2017, 9:06 PM

Thanks for the work! (I thought this was already landed last year :)

The change looks good to me (and I appreciate it as a step forward -- duplicated copies of cryptographic code is a big headache for us) overall.

(Totally optional: I wonder if the cryptoboot library could be folded into libstand? Or is there some specific reasons we have to create a new library instead? I'm asking because doing so would avoid building it multiple times, and the license of the source are all BSD compatible.)

I did not want to push it too hastily, one thing is the same concern about if it is good to have separate lib - the problem there is all this mess with options to pick/drop the features and for first step I just wanted to make it (libcryptoboot) very visible, so that maybe some better ideas will pop. The second issue is really about the build options (-Os versus -Oz). And then we have the alternate platforms next to x86:)

So in an sense, it still may be good idea to split this patch more. The libstand by itself does not really help about multiple builds, because we need 32-bit for bios and 64 (+32 in future?) for UEFI, + one for userboot (bhyve). But in any case, I did name it an experiment for purpose:)