Rework compat shims in ifioctl().
ClosedPublic
Actions

Authored by jhb on Apr 21 2021, 5:35 PM.

Details

Reviewers

kib
brooks

Commits

rGa88b9a2d1f6c: Rework compat shims in ifioctl().
rGd17e0940f79f: Rework compat shims in ifioctl().

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 created this revision.Apr 21 2021, 5:35 PM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptApr 21 2021, 5:35 PM

jhb requested review of this revision.Apr 21 2021, 5:35 PM

Harbormaster completed remote builds in B38759: Diff 87881.Apr 21 2021, 5:35 PM

jhb added a child revision: D29892: Add freebsd32 compat shims for SIOC[GS]DRVSPEC..Apr 21 2021, 5:35 PM

brooks accepted this revision.Apr 21 2021, 8:39 PM

This revision is now accepted and ready to land.Apr 21 2021, 8:39 PM

kib added inline comments.Apr 22 2021, 12:01 AM

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?

jhb added inline comments.Apr 22 2021, 6:17 PM

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.