Re: Reauthentication implementation flaw due to EINTR
Carl Fredrik Hammar, le Tue 29 Dec 2009 23:40:21 +0100, a écrit : > On Tue, Dec 29, 2009 at 11:10:00PM +0100, Samuel Thibault wrote: > > > > That's the same. To make it clearer, see attached patch. > > I meant in the other branch of the if (though for some reason I didn't > consider the server part's if). Err, what for? In that case, dead name notification isn't requested, is it? Or is it always requested by default and ports_interrupt_self_on_port_death() is just to change the behavior? (I'm a complete newbie for such parts of the Hurd...) Samuel
Re: Reauthentication implementation flaw due to EINTR
On Tue, Dec 29, 2009 at 11:10:00PM +0100, Samuel Thibault wrote:
>
> That's the same. To make it clearer, see attached patch.
I meant in the other branch of the if (though for some reason I didn't
consider the server part's if). Patch attatched for clarity.
Regards,
Fredrik
diff --git a/auth/auth.c b/auth/auth.c
index 3c5fa86..737fd98 100644
--- a/auth/auth.c
+++ b/auth/auth.c
@@ -294,20 +294,21 @@ S_auth_user_authenticate (struct authhandle *userauth,
s = hurd_ihash_find (&pending_servers, rendezvous);
if (s)
{
/* Found it! Extract the port. */
*newport = s->passthrough;
*newporttype = MACH_MSG_TYPE_MOVE_SEND;
/* Remove it from the pending list. */
hurd_ihash_locp_remove (&pending_servers, s->locp);
+ 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);
/* Give the server the auth port and wake the RPC up.
We need to add a ref in case the port dies. */
s->user = userauth;
ports_port_ref (userauth);
condition_signal (&s->wakeup);
mutex_unlock (&pending_lock);
mach_port_deallocate (mach_task_self (), rendezvous);
return 0;
Re: Reauthentication implementation flaw due to EINTR
Samuel Thibault, le Tue 29 Dec 2009 23:34:08 +0100, a écrit :
> > OK, I can use that for the client side. I don't really have a setup
> > for testing the auth server though. Do you use a sub-Hurd or something?
>
> I've never managed to find time to set up a sub-hurd, so I just debug
> with writes on the console :)
More precisely, I use
#include
#include
static void mylog(const char *fmt, ...)
{
static mach_port_t device = MACH_PORT_NULL;
if (device == MACH_PORT_NULL) {
mach_port_t priv, dev;
if (! get_privileged_ports(NULL, &priv)
&& ! device_open(priv, D_WRITE, "console", &dev))
device = dev;
}
if (device != MACH_PORT_NULL) {
char s[128];
int n;
va_list vap;
va_start(vap, fmt);
n = vsnprintf(s, sizeof(s), fmt, vap);
va_end(vap);
device_write(device, 0, 0, s, n, &n);
}
}
Samuel
Re: Reauthentication implementation flaw due to EINTR
Carl Fredrik Hammar, le Tue 29 Dec 2009 23:29:35 +0100, a écrit : > On Tue, Dec 29, 2009 at 10:55:41PM +0100, Samuel Thibault wrote: > > Carl Fredrik Hammar, le Tue 29 Dec 2009 22:48:26 +0100, a écrit : > > > It's still a mystery to me why ext2fs gets an EINTR without auth > > > explicitly returning EINTR... > > > > AIUI, the RPC gets aborted and ext2fs receives MACH_RCV_PORT_DIED, which > > _hurd_intr_rpc_mach_msg turns into EINTR. > > Wait, isn't the receive port in this case the auth port? This could > be bad... Oops, I meant MACH_RCV_TIMED_OUT above. > > BTW, reproducing the bug is really easy: > > > > while sudo date ; do : ; done > > OK, I can use that for the client side. I don't really have a setup > for testing the auth server though. Do you use a sub-Hurd or something? I've never managed to find time to set up a sub-hurd, so I just debug with writes on the console :) Samuel
Re: Reauthentication implementation flaw due to EINTR
On Tue, Dec 29, 2009 at 10:55:41PM +0100, Samuel Thibault wrote: > Carl Fredrik Hammar, le Tue 29 Dec 2009 22:48:26 +0100, a écrit : > > It's still a mystery to me why ext2fs gets an EINTR without auth > > explicitly returning EINTR... > > AIUI, the RPC gets aborted and ext2fs receives MACH_RCV_PORT_DIED, which > _hurd_intr_rpc_mach_msg turns into EINTR. Wait, isn't the receive port in this case the auth port? This could be bad... > BTW, reproducing the bug is really easy: > > while sudo date ; do : ; done OK, I can use that for the client side. I don't really have a setup for testing the auth server though. Do you use a sub-Hurd or something? Regards, Fredrik
Re: Reauthentication implementation flaw due to EINTR
Carl Fredrik Hammar, le Tue 29 Dec 2009 23:01:06 +0100, a écrit :
> On Mon, Dec 28, 2009 at 03:24:52AM +0100, Samuel Thibault wrote:
> > Carl Fredrik Hammar, le Sun 27 Dec 2009 20:56:21 +0100, a écrit :
> > > If the notification request is canceled before auth_user_authenticate
> > > returns there should be no problem.
> >
> > I tried a crude
> >
> > 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);
> >
> > after the complete if (hurd_condition_wait (&s.wakeup, &pending_lock)
> > statement, with no luck: ext2fs got an EINTR anyway.
>
> Err, shouldn't be in the user part after it finds the pending server if
> we want to do it before auth_user_authenticate returns?
That's the same. To make it clearer, see attached patch.
Samuel
--- auth.c.orig 2009-12-29 23:07:54.0 +0100
+++ auth.c 2009-12-29 23:08:41.0 +0100
@@ -328,18 +328,19 @@
condition_init (&u.wakeup);
ports_interrupt_self_on_port_death (userauth, rendezvous);
if (hurd_condition_wait (&u.wakeup, &pending_lock)
&& hurd_ihash_find (&pending_users, rendezvous))
/* We were interrupted; remove our record. */
{
hurd_ihash_locp_remove (&pending_users, u.locp);
err = EINTR;
}
+ 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);
if (! err)
{
/* The server RPC has set the port and signalled U.wakeup. */
*newport = u.passthrough;
*newporttype = MACH_MSG_TYPE_MOVE_SEND;
@@ -410,18 +411,19 @@
condition_init (&s.wakeup);
ports_interrupt_self_on_port_death (serverauth, rendezvous);
if (hurd_condition_wait (&s.wakeup, &pending_lock)
&& hurd_ihash_find (&pending_servers, rendezvous))
/* We were interrupted; remove our record. */
{
hurd_ihash_locp_remove (&pending_servers, s.locp);
err = EINTR;
}
+ 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);
if (err)
return err;
/* The user RPC has set the port (with a ref) and signalled S.wakeup. */
user = s.user;
Re: Reauthentication implementation flaw due to EINTR
On Mon, Dec 28, 2009 at 03:24:52AM +0100, Samuel Thibault wrote: > Carl Fredrik Hammar, le Sun 27 Dec 2009 20:56:21 +0100, a écrit : > > If the notification request is canceled before auth_user_authenticate > > returns there should be no problem. > > I tried a crude > > 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); > > after the complete if (hurd_condition_wait (&s.wakeup, &pending_lock) > statement, with no luck: ext2fs got an EINTR anyway. Err, shouldn't be in the user part after it finds the pending server if we want to do it before auth_user_authenticate returns? (I know you said something later about only testing this in the unpatched version, but it should work in the unpatched version if the problem is indeed the dead-name notification.) Regards, Fredrik
Re: Reauthentication implementation flaw due to EINTR
Carl Fredrik Hammar, le Tue 29 Dec 2009 22:48:26 +0100, a écrit : > It's still a mystery to me why ext2fs gets an EINTR without auth > explicitly returning EINTR... AIUI, the RPC gets aborted and ext2fs receives MACH_RCV_PORT_DIED, which _hurd_intr_rpc_mach_msg turns into EINTR. BTW, reproducing the bug is really easy: while sudo date ; do : ; done Samuel
Re: Reauthentication implementation flaw due to EINTR
Hi, On Mon, Dec 28, 2009 at 03:43:47AM +0100, Samuel Thibault wrote: > Carl Fredrik Hammar, le Sun 27 Dec 2009 22:24:24 +0100, a écrit : > > OK, I think I have a vague picture of what is going on: > > ports_interrupt_self_on_port_death > > ports_interrupt_self_on_notification > > ports_interrupt_rpc_on_notification, > > which requests notification (to the same port as > > auth_server_authenticate). > > But in another task (auth vs ext2fs). That's where I've still not found > what makes ext2fs return EINTR. > > > When rendezvous port dies we get the notification: > > Here we seemingly see ext2fs get the notification, but why? > > > ports_notify_server > > ports_do_mach_notify_dead_name > > ports_dead_name > > ports_interrupt_notified_rpcs > > hurd_thread_cancel (in glibc) > > _hurdsig_abort_rpcs, > > which does some funky stuff that seems to hijack any pending RPCs > > to make them return EINTR. I was hoping _hurdsig_abort_rpcs made auth_server_authenticate return EINTR somehow by interrupting auth_server_authenticate_reply (using thread_abort). But studying the code further it seems only interrupt the sending and not resend an EINTR message. (The only problem I can think of happening here is that the send isn't retried in which case ext2fs would hang.) So I looked further at libports which does a lot of bookkeeping of the pending RPCs, and so should be able to hijack the them to return EINTR. But none of the relevant code has any use of EINTR, so I don't think that's it either. It's still a mystery to me why ext2fs gets an EINTR without auth explicitly returning EINTR... Regards, Fredrik
Re: hurd/trans/hello.c changes
Hi, --- On Mon, Dec 28, 2009 at 4:13 AM, wrote: | I guess the hurd/ prefix is not necessary when building in-tree. As I | already said on IRC, the file was never meant to be compiled standalone. \-- Ok. --- | But perhaps it still works with the Prefix even in-tree... Could you | test that? \-- The hello translator builds, and works fine when built from the Hurd git sources. So, we will leave it at that. SK -- Shakthi Kannan http://www.shakthimaan.com
Re: cannot boot subhurd
On 09-12-29 下午5:42, olafbuddenha...@gmx.net wrote: > Hi, > > On Mon, Dec 28, 2009 at 11:45:50PM +0800, Da Zheng wrote: > >> I tried to boot subhurd but no matter how I try it, it just fails to >> boot. > >> #boot servers.boot /dev/hd1 > > Err... /dev/hd1? That's almost certainly wrong. You want to boot from a > partition, not from a drive... > > (I guess some kind of check for an invalid partition instead of a hang > would be nice though...) But it always worked before. I can also mount /dev/hd1 to my file system. 'settrans -a subhurd /hurd/ext2fs /dev/hd1' works on my system. antrik, have you used my modified subhurd? I wanted to try samuel's patch and see whether the patch can fix the bug in the modified subhurd (subhurd sometimes hangs). Can you try that for me if you have the time and if you have my modified subhurd? Thanks, Zheng Da
Re: cannot boot subhurd
Hi, On Mon, Dec 28, 2009 at 11:45:50PM +0800, Da Zheng wrote: > I tried to boot subhurd but no matter how I try it, it just fails to > boot. > #boot servers.boot /dev/hd1 Err... /dev/hd1? That's almost certainly wrong. You want to boot from a partition, not from a drive... (I guess some kind of check for an invalid partition instead of a hang would be nice though...) -antrik-
