Page MenuHomeFreeBSD

Deep copy tls_enable struct in copyin_tls_enable
ClosedPublic

Authored by rscheff on Mar 5 2024, 11:07 PM.
Tags
Referenced Files
Unknown Object (File)
Thu, May 2, 2:44 PM
Unknown Object (File)
Fri, Apr 26, 3:58 AM
Unknown Object (File)
Mon, Apr 22, 9:27 PM
Unknown Object (File)
Mon, Apr 22, 9:27 PM
Unknown Object (File)
Mon, Apr 22, 9:26 PM
Unknown Object (File)
Sun, Apr 21, 11:07 AM
Unknown Object (File)
Apr 14 2024, 6:08 PM
Unknown Object (File)
Apr 14 2024, 5:53 PM

Details

Summary

Doing a deep copy of the keys early allows users of the tls_enable structure to assume kernel memory.
This also enables the socket options to be set by kernel threads.

Taking over the commit process from cmiller_netapp.com after internal coordination.

Sponsored by: NetApp, Inc.
X-NetApp-PR: #79

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rscheff requested changes to this revision.Mar 6 2024, 6:34 PM
rscheff added a subscriber: rscheff.
rscheff added inline comments.
sys/kern/uipc_ktls.c
380

in ktls_destroy(), all crypto material is purged before freeing, by using zfree(). It would be prudent to do the same here for all these crypto keys.

This revision now requires changes to proceed.Mar 6 2024, 6:34 PM
rscheff edited reviewers, added: cmiller_netapp.com; removed: rscheff.
rscheff edited the summary of this revision. (Show Details)
rscheff removed a reviewer: rscheff.
  • clean before free all crypto material
This revision is now accepted and ready to land.Mar 7 2024, 3:45 PM
  • clean key material in the error path too.
This revision now requires review to proceed.Mar 7 2024, 9:15 PM

The use of zfree is fine and is the intended use case.

sys/kern/uipc_ktls.c
407

Should really be if (error != 0) in new code here and elsewhere.

442

Should check pointers against NULL explicitly, however, free(9) (just like free(3) in userland) handles NULL pointers just fine, so I would just remove all the checks and call zfree unconditionally instead.

sys/sys/ktls.h
216

I don't think you need the blank line after these, but they should be sorted alphabetically to match the rest of the prototypes. I would also probably add a ktls_ prefix to these two functions since the rest of the exported (public) API from uipc_ktls.c uses that prefix.

  • sort prototype definitions alphabetically

addressed all comments.

This revision is now accepted and ready to land.Mar 9 2024, 8:12 AM