Introduce vfcntl(), a va_list variant of fcntl()

[arichardson]

Summary

Certain libraries (e.g. epoll-shim) interpose libc's fcntl and there
is currently no portable way to interpose a variadic function without
relying on undefined behaviour. Adding a variant of fcntl that takes a
va_list instead of variadic arguments allows safely interposing fcntl.

While we can get away with reading a non-existent variadic argument on
all currently supported architectures (we just read garbage from the
next argument register or the stack), this is not possible for
architectures that pass pointers/integers in different registers or ones
that perform bounds checks on variadic arguments (e.g. CHERI). For
CHERI architectures, CheriBSD carries a downstream patch to only read
an argument for cmd values that expect one (and needs to take care to
read the correct type as well to avoid bounds errors).

[jrtc27]

there
is currently no portable way to interpose a variadic function without
relying on undefined behaviour

That's not entirely true, but it requires you to know how many arguments were passed in every possible case, what types they are, va_arg them in turn and call the interposed function with those arguments (or more). Whereas interposing a function that takes the va_list itself means you only need to interpret the arguments in the cases where you actually want to inspect at them.

lib/libsys/fcntl.2

42

Should be mentioned in the description?

[jrtc27]

lib/libsys/fcntl.2

42

In particular, you'll want to document the state of ap after the call like vprintf so the ownership, clobbering and whether va_end is needed from the caller are all defined.

[kib]

Could it be simplified by providing an fcntl variant that takes the third arg unconditionally, instead?

[arichardson]

In D46403#1057619, @kib wrote:

Could it be simplified by providing an fcntl variant that takes the third arg unconditionally, instead?

I would also be happy with that approach, just assumed this variant would be more flexible. Would the unconditional one look like this: fcntl2(int, int, __intptr_t)?

[arichardson]

In D46403#1057607, @jrtc27 wrote:

there
is currently no portable way to interpose a variadic function without
relying on undefined behaviour

That's not entirely true, but it requires you to know how many arguments were passed in every possible case, what types they are, va_arg them in turn and call the interposed function with those arguments (or more). Whereas interposing a function that takes the va_list itself means you only need to interpret the arguments in the cases where you actually want to inspect at them.

I can reword this "no portable way (without reimplementing the argument parsing logic)" if you prefer?

lib/libsys/fcntl.2

42

That's fair I can add those - I just assumed that va_end is always required for such calls unless specified otherwise.

[kib]

In D46403#1057647, @arichardson wrote:

In D46403#1057619, @kib wrote:

Could it be simplified by providing an fcntl variant that takes the third arg unconditionally, instead?

I would also be happy with that approach, just assumed this variant would be more flexible. Would the unconditional one look like this: fcntl2(int, int, __intptr_t)?

The actual syscall takes three args unconditionally, so there is no point in adding more complications IMO. By existing conventions, the function perhaps should be called fcntl3.

[arichardson]

In D46403#1057654, @kib wrote:

In D46403#1057647, @arichardson wrote:

In D46403#1057619, @kib wrote:

Could it be simplified by providing an fcntl variant that takes the third arg unconditionally, instead?

I would also be happy with that approach, just assumed this variant would be more flexible. Would the unconditional one look like this: fcntl2(int, int, __intptr_t)?

The actual syscall takes three args unconditionally, so there is no point in adding more complications IMO. By existing conventions, the function perhaps should be called fcntl3.

Actually, I just remembered this would not help with the interposing of fcntl since the interposer would still need to extract the variadic argument.

[jrtc27]

In D46403#1057695, @arichardson wrote:

In D46403#1057654, @kib wrote:

In D46403#1057647, @arichardson wrote:

In D46403#1057619, @kib wrote:

Could it be simplified by providing an fcntl variant that takes the third arg unconditionally, instead?

I would also be happy with that approach, just assumed this variant would be more flexible. Would the unconditional one look like this: fcntl2(int, int, __intptr_t)?

The actual syscall takes three args unconditionally, so there is no point in adding more complications IMO. By existing conventions, the function perhaps should be called fcntl3.

Actually, I just remembered this would not help with the interposing of fcntl since the interposer would still need to extract the variadic argument.

Well, fcntl would call fcntl3, and you can then interpose the latter, just as with vfcntl.

[arichardson]

In D46403#1057696, @jrtc27 wrote:

In D46403#1057695, @arichardson wrote:

In D46403#1057654, @kib wrote:

In D46403#1057647, @arichardson wrote:

In D46403#1057619, @kib wrote:

Could it be simplified by providing an fcntl variant that takes the third arg unconditionally, instead?

I would also be happy with that approach, just assumed this variant would be more flexible. Would the unconditional one look like this: fcntl2(int, int, __intptr_t)?

The actual syscall takes three args unconditionally, so there is no point in adding more complications IMO. By existing conventions, the function perhaps should be called fcntl3.

Actually, I just remembered this would not help with the interposing of fcntl since the interposer would still need to extract the variadic argument.

Well, fcntl would call fcntl3, and you can then interpose the latter, just as with vfcntl.

Fair, that would work - I am happy with either approach and will try to prototype the fcntl3 approach that in epoll-shim.