Re: Reauthentication implementation flaw due to EINTR

[Samuel Thibault]

Carl Fredrik Hammar, le Wed 30 Dec 2009 16:38:35 +0100, a écrit :
> On Wed, Dec 30, 2009 at 02:01:12PM +0100, Samuel Thibault wrote:
> > Carl Fredrik Hammar, le Wed 30 Dec 2009 12:40:02 +0100, a écrit :
> > >
> > > The notification has been requested by
> > > auth_server_authenticate which is waiting for auth_user_authenticate
> > > to arive.
> > 
> > Only when the user isn't arrived already.
> 
> Right, but we know it hasn't since the server has posted a pending
> operation, otherwise we'd be in the else part of the if.

Right.

> > > In the then part of the if statement, the user has found that the
> > > server is waiting for the condition to be signaled, which is right
> > > time to cancel the notification.
> > 
> > Err, isn't the notification bound to an RPC?
> > (really, I don't know anything about these and the Mach documentation
> > doesn't help me so much).
> 
> The low-level interface is bound to the port we want notifications about.

Ok.

I've tried the attached patch (against the current unpatched git head),
still no luck: no "auth EINTR" printed, but auth_server_authenticate
returned EINTR in ext2fs.

Samuel

--- auth.c.orig 2009-12-30 17:18:13.000000000 +0100
+++ auth.c      2009-12-30 17:24:25.000000000 +0100
@@ -306,6 +306,8 @@
       s->user = userauth;
       ports_port_ref (userauth);
 
+      mach_port_t old;
+      error_t err2 = mach_port_request_notification(mach_task_self(), 
rendezvous, MACH_NOTIFY_DEAD_NAME, 0, MACH_PORT_NULL, 
MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
       condition_signal (&s->wakeup);
       mutex_unlock (&pending_lock);
 
@@ -333,7 +335,10 @@
            {
              hurd_ihash_locp_remove (&pending_users, u.locp);
              err = EINTR;
+             ____mylog("auth EINTR\n");
            }
+         mach_port_t old;
+         error_t err2 = mach_port_request_notification(mach_task_self(), 
rendezvous, MACH_NOTIFY_DEAD_NAME, 0, MACH_PORT_NULL, 
MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
        }
       /* The server side has already removed U from the ihash table.  */
       mutex_unlock (&pending_lock);
@@ -393,6 +398,8 @@
       /* Give the user the new port and wake the RPC up.  */
       u->passthrough = newport;
 
+      mach_port_t old;
+      error_t err2 = mach_port_request_notification(mach_task_self(), 
rendezvous, MACH_NOTIFY_DEAD_NAME, 0, MACH_PORT_NULL, 
MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
       condition_signal (&u->wakeup);
       mutex_unlock (&pending_lock);
     }
@@ -415,7 +422,10 @@
            {
              hurd_ihash_locp_remove (&pending_servers, s.locp);
              err = EINTR;
+             ____mylog("auth EINTR\n");
            }
+         mach_port_t old;
+         error_t err2 = mach_port_request_notification(mach_task_self(), 
rendezvous, MACH_NOTIFY_DEAD_NAME, 0, MACH_PORT_NULL, 
MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
        }
       /* The user side has already removed S from the ihash table.  */
       mutex_unlock (&pending_lock);