bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Reauthentication implementation flaw due to EINTR


From: Carl Fredrik Hammar
Subject: Re: Reauthentication implementation flaw due to EINTR
Date: Sat, 26 Dec 2009 18:47:51 +0100
User-agent: Mutt/1.5.20 (2009-06-14)

Hi,

On Mon, Dec 21, 2009 at 08:43:12PM +0100, Samuel Thibault wrote:
> 
> I had been noticing odd issues with sudo when it calls setresuid &
> such, it took me some time to understand that there was a flaw in the
> reauthentication implementation:
> 
> sudo calls setresuid(), which calls setauth(), which (for each FD &
> such) allocates a rendez-vous port, calls io_reauthenticate() (RPC
> in the underlying FS which calls the auth_server_authenticate()
> RPC) and calls the auth_user_authenticate() RPC. These last two
> RPCs end up in auth, which uses the rendez-vous port passed along
> to make the match. Whichever arrives first leaves information and a
> condition variable for the other ; when the latter arrives, it fills its
> information and wakes the former.
> 
> The issue is that currently, once the user part gets its passthrough
> port from the server part, it returns immediately, and setauth() drops
> the rendez-vous port, which actually interrupts the server RPCs because
> the rendez-vous sender right becomes dead. Quite often scheduling makes
> it so that the user is not so fast and the server has time to finish its
> duties, but due to the high usage of setresuid in sudo, one every few
> tens of sudo calls fail.

Is it the code below from S_auth_server_authenticate the problem?

  /* Store the new port and wait for the user RPC to wake us up.  */
  s.passthrough = newport;
  condition_init (&s.wakeup);
  ports_interrupt_self_on_port_death (serverauth, rendezvous);
  if (hurd_condition_wait (&s.wakeup, &pending_lock))
    /* We were interrupted; remove our record.  */
    {
      hurd_ihash_locp_remove (&pending_servers, s.locp);
      err = EINTR;
    }

That is, does hurd_condition_wait get canceled even though the condition
was signaled before it was canceled?

> The fix I'll use at least on the Debian buildds for now is to make the
> auth_user_authenticate() RPC always wait for auth_server_authenticate()
> to have called auth_server_authenticate_reply() before returning. I've
> been running that in a tight loop the whole afternoon with no issue, so
> at least it seems to work much better. However, I'd prefer to make sure
> that it works _always_ :)
>
> So my question is: is it sufficient to make the user part wait for
> auth_server_authenticate_reply() call completion before freeing the
> rendez-vous port, to make sure that auth_server_authenticate() will
> never return EINTR because of the death of the rendez-vous port?  Of
> course, the rendez-vous port can become dead in the io_reauthenticate()
> RPC, but that shouldn't be a problem.

If what I said above is correct then this would indeed fix it.
The question is if there are better ways to fix it.

Perhaps trying to determine if the condition was signaled before the
interrupt?  Though I don't know if can be interrupted for other reasons
and should always return EINTR in those cases.  Perhaps the mistake is
using interrupts as a signal for port deaths in the first place?

> And bonus question: are there other places where we have such
> rendez-vous port which might become dead too early and that would need
> the same fix?

Don't know.

Regards,
  Fredrik




reply via email to

[Prev in Thread] Current Thread [Next in Thread]