Page MenuHomeFreeBSD

Use explicit_bzero() instead of bzero() for hash context state.
Needs ReviewPublic

Authored by jhb on Thu, Jun 25, 12:37 AM.

Details

Summary

While here, add a missing zero for temporary hash context in
abd_checksum_edonr_native().

Test Plan
  • compile tested only

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 31943
Build 29494: arc lint + arc unit

Event Timeline

jhb created this revision.Thu, Jun 25, 12:37 AM
jhb requested review of this revision.Thu, Jun 25, 12:37 AM
jhb added a comment.Thu, Jun 25, 12:40 AM

Using explicit_bzero vs bzero is largely for readability, though it can also prevent "helpful" compiler optimizations if bzero is mapped to a memset the compiler knows it can elide. The compiler probably won't elide the bzero before calling kmem_free but might elide ones at the end of functions.

I'm not tied to committing this to in-tree ZFS too much, just found these while doing a tree sweep for places that could have used zfree and then checking related code. Fixing it in ZoF is probably more useful / important.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/edonr_zfs.c
60

It is probably worth checking that this is fixed in ZoF.

Actually, my expectation was that the _Final methods shall destroy the context descriptor (and thus the bzero's are not necessary) like all other cryptographic hash methods, but apparently we are supporting the use of skein.h directly (!), and it doesn't do the bzero'ing.

I haven't pursued what was the motivation of this design choice and personally I'd prefer getting the code to use the more standardized SKEIN512_* API instead, but since ZFS is currently the only skein consumer in kernel and the change as-is is at minimum an improvement to the status quo in my opinion, consider this as a "LGTM".

jhb added reviewers: avg, mav.Fri, Jun 26, 12:00 AM