On Tue, Sep 10, 2024 at 4:49 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Thu, Aug 15, 2024 at 4:00 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > > On Thu, Aug 15, 2024 at 9:46 PM David Howells <dhowells@xxxxxxxxxx> wrote: > > > Jann Horn <jannh@xxxxxxxxxx> wrote: > > > > > > > Rewrite keyctl_session_to_parent() to run task work on the parent > > > > synchronously, so that any errors that happen in the task work can be > > > > plumbed back into the syscall return value in the child. > > > > > > The main thing I worry about is if there's a way to deadlock the child and the > > > parent against each other. vfork() for example. > > > > Yes - I think it would work fine for scenarios like using > > KEYCTL_SESSION_TO_PARENT from a helper binary against the shell that > > launched the helper (which I think is the intended usecase?), but > > there could theoretically be constellations where it would cause an > > (interruptible) hang if the parent is stuck in > > uninterruptible/killable sleep. > > > > I think vfork() is rather special in that it does a killable wait for > > the child to exit or execute; and based on my understanding of the > > intended usecase of KEYCTL_SESSION_TO_PARENT, I think normally > > KEYCTL_SESSION_TO_PARENT would only be used by a child that has gone > > through execve? > > Where did we land on all of this? Unless I missed a thread somewhere, > it looks like the discussion trailed off without any resolution on if > we are okay with a potentially (interruptible) deadlock? As a potential tweak to this, what if we gave up on the idea of returning the error code so we could avoid the signal deadlock issue? I suppose there could be an issue if the parent was expecting/depending on keyring change from the child, but honestly, if the parent is relying on the kernel keyring and spawning a child process without restring the KEYCTL_SESSION_TO_PARENT then the parent really should be doing some sanity checks on the keyring after the child returns anyway. I'm conflicted on the best way to solve this problem, but I think we need to fix this somehow as I believe the current behavior is broken ... -- paul-moore.com
