Hurd: Implement chdir support in posix_spawn

Message ID 20181108024849.nnu6d3u6xgfyru7o@function
State Committed, archived
Headers

Commit Message

Samuel Thibault Nov. 8, 2018, 2:48 a.m. UTC
  Florian Weimer, le mer. 07 nov. 2018 12:37:05 +0100, a ecrit:
> This fixes build-many-glibcs.py on i686-gnu.
> 
> Note that I don't really know what I'm doing here, and I don't have a
> way to check this code.

The idea was correct, only the details were not enough.  Here is the
changes I made on top of your patch to fix it. How do you prefer to
proceed to commit your patch and mine?

Samuel
diff --git a/sysdeps/mach/hurd/spawni.c b/sysdeps/mach/hurd/spawni.c
index b554ccc646..ff9eb9580a 100644
--- a/sysdeps/mach/hurd/spawni.c
+++ b/sysdeps/mach/hurd/spawni.c
@@ -113,6 +113,9 @@ __spawni (pid_t *pid, const char *file,
   struct hurd_userlink *ulink_dtable = NULL;
   struct hurd_sigstate *ss;
 
+  /* Child current working dir */
+  file_t ccwdir = MACH_PORT_NULL;
+
   /* For POSIX_SPAWN_RESETIDS, this reauthenticates our root/current
      directory ports with the new AUTH port.  */
   file_t rcrdir = MACH_PORT_NULL, rcwdir = MACH_PORT_NULL;
@@ -123,16 +126,25 @@ __spawni (pid_t *pid, const char *file,
       if (*result != MACH_PORT_NULL)
 	return 0;
       ref = __mach_reply_port ();
-      err = HURD_PORT_USE
-	(&_hurd_ports[which],
-	 ({
-	   err = __io_reauthenticate (port, ref, MACH_MSG_TYPE_MAKE_SEND);
-	   if (!err)
-	     err = __auth_user_authenticate (auth,
-					     ref, MACH_MSG_TYPE_MAKE_SEND,
-					     result);
-	   err;
-	 }));
+      if (which == INIT_PORT_CWDIR && ccwdir != MACH_PORT_NULL)
+	{
+	  err = __io_reauthenticate (ccwdir, ref, MACH_MSG_TYPE_MAKE_SEND);
+	  if (!err)
+	    err = __auth_user_authenticate (auth,
+					    ref, MACH_MSG_TYPE_MAKE_SEND,
+					    result);
+	}
+      else
+	err = HURD_PORT_USE
+	  (&_hurd_ports[which],
+	   ({
+	     err = __io_reauthenticate (port, ref, MACH_MSG_TYPE_MAKE_SEND);
+	     if (!err)
+	       err = __auth_user_authenticate (auth,
+					       ref, MACH_MSG_TYPE_MAKE_SEND,
+					       result);
+	     err;
+	   }));
       __mach_port_destroy (__mach_task_self (), ref);
       return err;
     }
@@ -177,6 +189,14 @@ __spawni (pid_t *pid, const char *file,
 	    return (reauthenticate (INIT_PORT_CWDIR, &rcwdir)
 		    ?: (*operate) (rcwdir));
 	  }
+      else
+	switch (which)
+	  {
+	  case INIT_PORT_CWDIR:
+	    if (ccwdir != MACH_PORT_NULL)
+	      return (*operate) (ccwdir);
+	    break;
+	  }
       assert (which != INIT_PORT_PROC);
       return _hurd_ports_use (which, operate);
     }
@@ -207,13 +227,7 @@ __spawni (pid_t *pid, const char *file,
     }
   auto error_t child_chdir (const char *name)
     {
-      if (rcwdir != MACH_PORT_NULL)
-	{
-	  __mach_port_deallocate (__mach_task_self (), rcwdir);
-	  rcwdir = MACH_PORT_NULL;
-	}
-
-      /* See _hurd_change_directory_port_from_name.  */
+      file_t new_ccwdir;
 
       /* Append trailing "/." to directory name to force ENOTDIR if
 	 it's not a directory and EACCES if we don't have search
@@ -235,7 +249,15 @@ __spawni (pid_t *pid, const char *file,
 	  lookup = n;
 	}
 
-      return child_lookup (lookup, 0, 0, &rcwdir);
+      error_t err = child_lookup (lookup, 0, 0, &new_ccwdir);
+      if (!err)
+	{
+	  if (ccwdir != MACH_PORT_NULL)
+	    __mach_port_deallocate (__mach_task_self (), ccwdir);
+	  ccwdir = new_ccwdir;
+	}
+
+      return err;
     }
 
 
@@ -744,6 +783,11 @@ __spawni (pid_t *pid, const char *file,
 		ports[i] = rcwdir;
 		continue;
 	      }
+	    if (ccwdir != MACH_PORT_NULL)
+	      {
+		ports[i] = ccwdir;
+		continue;
+	      }
 	    break;
 	  }
 	ports[i] = _hurd_port_get (&_hurd_ports[i], &ulink_ports[i]);
@@ -784,6 +828,8 @@ __spawni (pid_t *pid, const char *file,
 	  case INIT_PORT_CWDIR:
 	    if (flags & POSIX_SPAWN_RESETIDS)
 	      continue;
+	    if (ccwdir != MACH_PORT_NULL)
+	      continue;
 	    break;
 	  }
 	_hurd_port_free (&_hurd_ports[i], &ulink_ports[i], ports[i]);
@@ -809,6 +855,8 @@ __spawni (pid_t *pid, const char *file,
     }
   __mach_port_deallocate (__mach_task_self (), auth);
   __mach_port_deallocate (__mach_task_self (), proc);
+  if (ccwdir != MACH_PORT_NULL)
+    __mach_port_deallocate (__mach_task_self (), ccwdir);
   if (rcrdir != MACH_PORT_NULL)
     __mach_port_deallocate (__mach_task_self (), rcrdir);
   if (rcwdir != MACH_PORT_NULL)
  

Comments

Florian Weimer Nov. 9, 2018, 1:05 p.m. UTC | #1
* Samuel Thibault:

> Florian Weimer, le mer. 07 nov. 2018 12:37:05 +0100, a ecrit:
>> This fixes build-many-glibcs.py on i686-gnu.
>> 
>> Note that I don't really know what I'm doing here, and I don't have a
>> way to check this code.
>
> The idea was correct, only the details were not enough.  Here is the
> changes I made on top of your patch to fix it. How do you prefer to
> proceed to commit your patch and mine?

Please commit it with you as the author.  You can add my name to the
ChangeLog update if you want, but that's not necessary.

Is it okay if I commit fchdir support without the Hurd part?  I want to
add that next, but that will break the Hurd build again.

Thanks,
Florian
  
Samuel Thibault Nov. 10, 2018, 11:01 a.m. UTC | #2
Hello,

Florian Weimer, le ven. 09 nov. 2018 14:05:30 +0100, a ecrit:
> Please commit it with you as the author.  You can add my name to the
> ChangeLog update if you want, but that's not necessary.

Ok, done so, thanks!

> Is it okay if I commit fchdir support without the Hurd part?  I want to
> add that next, but that will break the Hurd build again.

No problem, I'll handle it :)

Samuel
  

Patch

diff --git a/sysdeps/mach/hurd/spawni.c b/sysdeps/mach/hurd/spawni.c
index b554ccc646..ff9eb9580a 100644
--- a/sysdeps/mach/hurd/spawni.c
+++ b/sysdeps/mach/hurd/spawni.c
@@ -113,6 +113,9 @@  __spawni (pid_t *pid, const char *file,
   struct hurd_userlink *ulink_dtable = NULL;
   struct hurd_sigstate *ss;
 
+  /* Child current working dir */
+  file_t ccwdir = MACH_PORT_NULL;
+
   /* For POSIX_SPAWN_RESETIDS, this reauthenticates our root/current
      directory ports with the new AUTH port.  */
   file_t rcrdir = MACH_PORT_NULL, rcwdir = MACH_PORT_NULL;
@@ -123,16 +126,25 @@  __spawni (pid_t *pid, const char *file,
       if (*result != MACH_PORT_NULL)
 	return 0;
       ref = __mach_reply_port ();
-      err = HURD_PORT_USE
-	(&_hurd_ports[which],
-	 ({
-	   err = __io_reauthenticate (port, ref, MACH_MSG_TYPE_MAKE_SEND);
-	   if (!err)
-	     err = __auth_user_authenticate (auth,
-					     ref, MACH_MSG_TYPE_MAKE_SEND,
-					     result);
-	   err;
-	 }));
+      if (which == INIT_PORT_CWDIR && ccwdir != MACH_PORT_NULL)
+	{
+	  err = __io_reauthenticate (ccwdir, ref, MACH_MSG_TYPE_MAKE_SEND);
+	  if (!err)
+	    err = __auth_user_authenticate (auth,
+					    ref, MACH_MSG_TYPE_MAKE_SEND,
+					    result);
+	}
+      else
+	err = HURD_PORT_USE
+	  (&_hurd_ports[which],
+	   ({
+	     err = __io_reauthenticate (port, ref, MACH_MSG_TYPE_MAKE_SEND);
+	     if (!err)
+	       err = __auth_user_authenticate (auth,
+					       ref, MACH_MSG_TYPE_MAKE_SEND,
+					       result);
+	     err;
+	   }));
       __mach_port_destroy (__mach_task_self (), ref);
       return err;
     }
@@ -177,6 +189,14 @@  __spawni (pid_t *pid, const char *file,
 	    return (reauthenticate (INIT_PORT_CWDIR, &rcwdir)
 		    ?: (*operate) (rcwdir));
 	  }
+      else
+	switch (which)
+	  {
+	  case INIT_PORT_CWDIR:
+	    if (ccwdir != MACH_PORT_NULL)
+	      return (*operate) (ccwdir);
+	    break;
+	  }
       assert (which != INIT_PORT_PROC);
       return _hurd_ports_use (which, operate);
     }
@@ -207,13 +227,7 @@  __spawni (pid_t *pid, const char *file,
     }
   auto error_t child_chdir (const char *name)
     {
-      if (rcwdir != MACH_PORT_NULL)
-	{
-	  __mach_port_deallocate (__mach_task_self (), rcwdir);
-	  rcwdir = MACH_PORT_NULL;
-	}
-
-      /* See _hurd_change_directory_port_from_name.  */
+      file_t new_ccwdir;
 
       /* Append trailing "/." to directory name to force ENOTDIR if
 	 it's not a directory and EACCES if we don't have search
@@ -235,7 +249,15 @@  __spawni (pid_t *pid, const char *file,
 	  lookup = n;
 	}
 
-      return child_lookup (lookup, 0, 0, &rcwdir);
+      error_t err = child_lookup (lookup, 0, 0, &new_ccwdir);
+      if (!err)
+	{
+	  if (ccwdir != MACH_PORT_NULL)
+	    __mach_port_deallocate (__mach_task_self (), ccwdir);
+	  ccwdir = new_ccwdir;
+	}
+
+      return err;
     }
 
 
@@ -744,6 +783,11 @@  __spawni (pid_t *pid, const char *file,
 		ports[i] = rcwdir;
 		continue;
 	      }
+	    if (ccwdir != MACH_PORT_NULL)
+	      {
+		ports[i] = ccwdir;
+		continue;
+	      }
 	    break;
 	  }
 	ports[i] = _hurd_port_get (&_hurd_ports[i], &ulink_ports[i]);
@@ -784,6 +828,8 @@  __spawni (pid_t *pid, const char *file,
 	  case INIT_PORT_CWDIR:
 	    if (flags & POSIX_SPAWN_RESETIDS)
 	      continue;
+	    if (ccwdir != MACH_PORT_NULL)
+	      continue;
 	    break;
 	  }
 	_hurd_port_free (&_hurd_ports[i], &ulink_ports[i], ports[i]);
@@ -809,6 +855,8 @@  __spawni (pid_t *pid, const char *file,
     }
   __mach_port_deallocate (__mach_task_self (), auth);
   __mach_port_deallocate (__mach_task_self (), proc);
+  if (ccwdir != MACH_PORT_NULL)
+    __mach_port_deallocate (__mach_task_self (), ccwdir);
   if (rcrdir != MACH_PORT_NULL)
     __mach_port_deallocate (__mach_task_self (), rcrdir);
   if (rcwdir != MACH_PORT_NULL)