Page MenuHomeFreeBSD

rpc/xdr.h: make xdrproc_t always take two arguments
AcceptedPublic

Authored by brooks on Thu, Jan 22, 10:12 AM.
Tags
None
Referenced Files
F142765240: D54824.id.diff
Fri, Jan 23, 8:19 AM
F142759566: D54824.diff
Fri, Jan 23, 6:10 AM
F142724431: D54824.diff
Thu, Jan 22, 5:31 PM
F142717472: D54824.diff
Thu, Jan 22, 3:15 PM
F142712385: D54824.id170229.diff
Thu, Jan 22, 1:57 PM
Subscribers

Details

Reviewers
kib
markj
Group Reviewers
cheri
Summary

The type of xdrproc_t is clearly defined in the comments as a function
to arguments, an XDR * and a void * (sometimes spelled caddr_t). It was
initialy defined as:
typedef bool_t (*xdrproc_t)();

At some point people started giving it a non-empty argument list.
Unfortunatly, there has been widespread disagreement about how arguments
are passed. There seems to have been a widespread view that it should
be allowed to pass three argument function pointer to xdrproc_t. Most
notable is xdr_string which takes a maximum length parameter. This lead
to all sorts of prototypes (all of which have been present in the
FreeBSD source tree):

FreeBSD userspace (nominally from tirpc, but seemingly local):
typedef bool_t (*xdrproc_t)(XDR *, ...);
FreeBSD kernel, glibc:
typedef bool_t (*xdrproc_t)(XDR *, void *, ...);
rcp/xdr.h with _KERNEL defined (not used?):
typedef bool_t (*xdrproc_t)(XDR *, void *, u_int);
gssrpc (in krb5) and Linux kernel:
typedef bool_t (*xdrproc_t)(XDR *, void *);

For two argument functions on current ABIs, these all equivalent as
these arguments are passed in registers regardless of decleration, but
we end up with two problems.

  • xdr_free((xdrproc_t)xdr_string, ...) calls xdr_string with no third argument and (at least on FreeBSD) may fail to free memory if the string is shorter than the value lying around in the third argument register. There are no instance of this in tree, but I found some with Debian code search, in particular in OpenAFS.
  • Under CheriABI, variadic arguments are passed in a seperate, bounded array so theses prototypes aren't equilvalent.

The reality is that that xdr_string should not be cast to xdrproc_t and
xdr_wrapstring should be used instead so we do not need to support this
case. Instances of the former behavior are now extremely rare.

With this change we bring FreeBSD in line with gssrpc and the Linux
Kernel. Warnings about casts should now be correct and should be fixed.

Bump __FreeBSD_version as some software required adaptation if it is
declaring functions to cast to xdrproc_t. Update OpenZFS's workaround
of this historic mess accordingly.

Effort: CHERI upstreaming
Sponsored by: Innovate UK

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 70055
Build 66938: arc lint + arc unit