Page MenuHomeFreeBSD

Simplify compat shims for /dev/crypto.
ClosedPublic

Authored by jhb on Aug 24 2020, 11:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 26 2024, 2:03 PM
Unknown Object (File)
Feb 20 2024, 4:27 PM
Unknown Object (File)
Dec 20 2023, 7:59 AM
Unknown Object (File)
Nov 13 2023, 2:14 PM
Unknown Object (File)
Nov 7 2023, 1:12 AM
Unknown Object (File)
Oct 19 2023, 12:39 AM
Unknown Object (File)
Aug 29 2023, 2:23 AM
Unknown Object (File)
Aug 15 2023, 2:58 PM
Subscribers

Details

Summary
  • Make session handling always use the CIOGSESSION2 structure. CIOGSESSION requests use a thunk similar to COMPAT_FREEBSD32 session requests. This permits the ioctl handler to use the 'crid' field uncoditionally.
  • Move COMPAT_FREEBSD32 handling out of the main ioctl handler body and instead do conversions in/out of thunk structures in dedicated blocks at the start and end of the ioctl function.

Sponsored by: Netflix

Test Plan
  • tested with cryptocheck -d soft -a all -z

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 33182
Build 30531: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Aug 24 2020, 11:57 PM
jhb created this revision.

This looks ok to me, my comments are just nit-picking.

sys/opencrypto/cryptodev.c
409

It makes me slightly uneasy that we're passing void pointers to these functions which have similar names but operate on different-sized structures. Explicit pointer casting might be worth doing here, and you could use the stack variables names. For example, the lines above could be:

session2_op_from_32((struct session2_op32 *)data32, &sopc);
data = &sopc;
742

Why not use else if?

828

It's kind of weird that this is not conditional on error == 0, but I see that it was that way before.

This revision is now accepted and ready to land.Aug 25 2020, 2:13 PM
sys/opencrypto/cryptodev.c
409

I can change it. I mostly did this so that the code followed a more regular pattern. It was easier to copy and paste from one to generate the next. I do the same at the end of always using (data, data32) for similar reasons.

I do think that the casting of data32 is a bit verbose, but I could use the stack variable at least and getting the type right for at least one of the arguments will probably catching any meaningful mismatches.

I should also probably put the compat structures in a union.

742

I guess I could. I had probably been trying to isolate the compat shim here from the regular body of the code, but it can't hurt.

828

Agreed. I'd actually like to axe all the asym crypto stuff. OpenSSL 1.1.0 and later don't use it, it's under-documented, and the only implementation is for some MIPS parts no one probably uses. I do think qat(4) could actually do it in theory, but meh.

sys/opencrypto/cryptodev.c
409

I'm fine with the change as it is, really just noting that I had to double check carefully when reading this bit.

  • Avoid void types for the first set of conversion routines.
This revision now requires review to proceed.Aug 26 2020, 8:14 PM
sys/opencrypto/cryptodev.c
409

I went ahead and changed these to make it easier to avoid mistakes in the future.

I also added a union to conserve stack space.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 26 2020, 9:17 PM
This revision was automatically updated to reflect the committed changes.