Page MenuHomeFreeBSD

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

Authored by jhb on Jun 25 2020, 12:37 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 9 2024, 3:45 PM
Unknown Object (File)
Dec 22 2023, 5:59 AM
Unknown Object (File)
Dec 20 2023, 4:58 AM
Unknown Object (File)
Sep 5 2023, 11:51 PM
Unknown Object (File)
Aug 13 2023, 6:31 AM
Unknown Object (File)
Aug 2 2023, 6:24 PM
Unknown Object (File)
Jun 9 2023, 3:27 PM
Unknown Object (File)
May 22 2023, 12:06 AM
Subscribers

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
Tests Skipped
Build Status
Buildable 31943
Build 29494: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Jun 25 2020, 12:37 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".