src: add uv_setxid() set of functions

[santigimeno]

The only reason to add these wrappers is to allow embedders to prevent the privileged-dropped processes to perform privileged operations when using io_uring on linux.

Refs: GHSA-vr4q-vx84-9g5x

[santigimeno]

This is my attempt to address #4416. Also, it might allow to re-enable io_uring in Node.js. If this is smthg we're ok with I'll document it. Let me know what you think. /cc @bnoordhuis

[bnoordhuis]

I don't think this completely solves the Node.js issue because there's still the following edge case:

1. Submit file operation
2. Call uv_setuid()
3. File operation completes

That performs the operation with the credentials from before the credentials change. The threadpool also has that issue but to a lesser extent (if the worker thread picks it off the queue after the credentials change, it's definitely using the new credentials.)

Aside from that, I wouldn't phrase the API in such a Unix-specific way, just e.g. uv_credentials_changed().

[santigimeno]

I don't think this completely solves the Node.js issue because there's still the following edge case:

1. Submit file operation
2. Call uv_setuid()
3. File operation completes

That performs the operation with the credentials from before the credentials change.

Care to elaborate why is that an issue? Wouldn't it be the expected behavior if we were using sync fs calls directly (the credentials would be changed after the fact)? Or am I missing smthg?

Aside from that, I wouldn't phrase the API in such a Unix-specific way, just e.g. uv_credentials_changed().

I thought that from the embedder's pov would feel more natural just replace the setxid() calls with the uv_setxid() counterparts.

[bnoordhuis]

Sorry for the delay, been sick and afterwards had a lot of catching up to do with Real World™ stuff.

the credentials would be changed after the fact

My point there is that you get an observable difference between io_uring and the thread pool (but, again, the thread pool is not immune to the problem, just less so.)

I thought that from the embedder's pov would feel more natural just replace the setxid() calls with the uv_setxid() counterparts

I can see that but setxid doesn't really translate to capability-based environments like fuchsia. I'd rather avoid us painting ourselves into a corner.

[santigimeno]

Sorry for the delay, been sick and afterwards had a lot of catching up to do with Real World™ stuff.

Sorry to hear. Hope all is well now.

My point there is that you get an observable difference between io_uring and the thread pool (but, again, the thread pool is not immune to the problem, just less so.)

Ok, I think I understand your point now. So with io_uring (and sync calls) we would get consistent behavior whereas with the thread poll don't? If that's the case, not sure how big of a problem is that: it doesn't seem unreasonable to think that if you change the credentials after making the fs call, those credentials won't apply to that operation.

I can see that but setxid doesn't really translate to capability-based environments like fuchsia. I'd rather avoid us painting ourselves into a corner.

Fair enough: I didn't know about those environments.

Regardless, let me know if you think this approach is worth pursuing. Tbh, not very invested in this unless we're able to solve the other io_uring perf issues we've been observing.

[bnoordhuis]

Sorry to hear. Hope all is well now.

Thanks! I'm fine again, smashing boulders and scaling routes in my climbing gym as if gravity is a mere afterthought.

(I may be slightly exaggerating.)

Tbh, not very invested in this unless we're able to solve the other io_uring perf issues we've been observing.

Yeah.... so far, io_uring has been - at best - a qualified success. What do you think of flipping the default to off, and having people opt-in through an environment variable or uv_loop_configure(loop, UV_LOOP_USE_IO_URING)?

We can still have a uv_loop_credentials_changed(loop) notification for users like node.js, or repurpose uv_loop_configure with another new flag.

[saghul]

. What do you think of flipping the default to off, and having people opt-in through an environment variable or uv_loop_configure(loop, UV_LOOP_USE_IO_URING)?

Sounds reasonable. Would this apply only to the thread pool use of uring or also to the syscall batching use?

[bnoordhuis]

epoll_ctl batching isn't affected by credentials changes (AFAIK) and hasn't been problematic in general so I'd just keep that unless there are strong reasons not to.

edit: maybe with s/UV_LOOP_USE_IO_URING/UV_LOOP_USE_IO_URING_SQPOLL/

[santigimeno]

I can get behind this. Unfortunately using SQPOLL for a general purpose library ended up not being the best choice. I'll try to create a couple of PR's (one for the UV_LOOP_USE_IO_URING_SQPOLL and another for the credentials thing)

[bnoordhuis]

@santigimeno with #4492 merged, can this be closed, or...?

[santigimeno]

I'd keep it open. I'll add the credentials fn after the release.