Page MenuHomeFreeBSD

Rework compat shims in ifioctl().
ClosedPublic

Authored by jhb on Apr 21 2021, 5:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 21, 2:36 AM
Unknown Object (File)
Thu, Mar 21, 2:36 AM
Unknown Object (File)
Thu, Mar 21, 2:36 AM
Unknown Object (File)
Thu, Mar 21, 2:36 AM
Unknown Object (File)
Thu, Mar 21, 2:21 AM
Unknown Object (File)
Tue, Mar 19, 5:16 AM
Unknown Object (File)
Tue, Mar 12, 9:21 AM
Unknown Object (File)
Thu, Mar 7, 11:31 PM

Details

Summary

Centralize logic for handling compat ioctls into two blocks of code at
the start and end of the ioctl routine. This avoids the conversion
logic being spread out both in multiple blocks in ifioctl as well as
various helper functions.

Obtained from: CheriBSD
Sponsored by: DARPA

Diff Detail

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

Event Timeline

jhb requested review of this revision.Apr 21 2021, 5:35 PM
This revision is now accepted and ready to land.Apr 21 2021, 8:39 PM
sys/net/if.c
2926–2927

Why do you need the initialization there?

2927–2934

this assumes that '0' is not a value for some of ioctls.

I think we need something along the lines of _IOC_INVALID (IOC_VOID | IOC_INOUT) in ioccom.h

3079–3092

error != 0

3083

ifc32 is already initialized

3088

ifmr32 is already initialized, no?

sys/net/if.c
2927–2934

this assumes that '0' is not a value for some of ioctls.

I think we need something along the lines of _IOC_INVALID (IOC_VOID | IOC_INOUT) in ioccom.h

Hmmm, I would not oppose such a value, though I think 0 is in fact invalid as an ioctl value.

2981

I suppose I could add a default case here which initializes saved_cmd and saved_data.

3083

While it is, I don't think the compiler is smart enough to realize that it is.

jhb marked an inline comment as done.May 4 2021, 5:56 PM
jhb added inline comments.
sys/net/if.c
2926–2927

Why do you need the initialization there?

Actually, after further thinking, I could just always initialize these to 'cmd' and 'data'. It doesn't hurt for them to be set for non-compat ioctls (the switch at the end just wouldn't match anything for non-compat ioctls). I can do that down below before the first switch though rather than doing it during assignment, though doing it during assignment here would actually be consistent with Warner's recent language about read-only variables that are never changed in a block.

jhb marked 2 inline comments as done.May 4 2021, 5:57 PM
jhb added inline comments.
sys/net/if.c
3083

Turns out, GCC and clang are both fine with this, so I'll drop these.

  • Update for review feedback.
This revision now requires review to proceed.May 4 2021, 6:25 PM
This revision is now accepted and ready to land.May 4 2021, 8:42 PM
This revision was automatically updated to reflect the committed changes.