shadow-maint/shadow

pidfd for `new?idmap`

vinipsmaker opened this issue · 4 comments

Would it be possible to offer an interface where we pass a pidfd instead of a PID number to newuidmap and newgidmap? For the program I designed (I can provide more info if requested), I can't really reserve PIDs (reaping must be automatic through SIGCHLD) until newuidmap is done, so there's a chance for a race that is unlikely, but I'd like to avoid anyway. I do have pidfds (and pidns fds) so if it's possible for me to feed them to newuidmap that would solve my problem.

Good question. I don't know whether you can from a pidfd_open fd return valueto the corresponding /proc/pid O_PATH fd, or just immediately do an openat(pidfd_retval, "uid_map"), which is what we'd need to be able to to do. But if not, I bet we can add that support.

pinging @brauner

I don't know whether you can from a pidfd_open fd return valueto the corresponding /proc/pid O_PATH fd, or just immediately do an openat(pidfd_retval, "uid_map")

Just to be clear, I can pass any fd that newuidmap might need. Right now I open fds on the child using /proc/self/* paths and send them to the parent through a pre-shared/inherited socket. That still is race-free. I have a race-free interface to open any file on /proc/*. The problem really is feeding a fd to newuidmap.

Ah - then absolutely we can do something. What would be the problem with simply passing --pidfd=5, meaning the process doing exec has the /prod/pid fd open in fd 5, instead of the usual pid?

So,

newuidmap pid uid loweruid count [uid loweruid count [ ... ]]
newuidmap --pidfd=5 uid loweruid count [uid loweruid count [ ... ]]

Caller just has to make sure not to the pidfd it with CLOEXEC, right?

I think --pidfd would work for me, yes. However could the feature be done in a separate branch before it's merged to master? I'm not sure I followed your proposal fully so I'd like a chance to review it.