[PATCH] Hurd: Implement chdir support in posix_spawn

[PATCH] Hurd: Implement chdir support in posix_spawn

[Florian Weimer]

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.

Thanks,
Florian

2018-11-07  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/mach/hurd/spawni.c (__spawni): Add nested function
	child_chdir.  Handle spawn_do_chdir.

diff --git a/sysdeps/mach/hurd/spawni.c b/sysdeps/mach/hurd/spawni.c
index 9351c13e56..b554ccc646 100644
--- a/sysdeps/mach/hurd/spawni.c
+++ b/sysdeps/mach/hurd/spawni.c
@@ -205,6 +205,38 @@ __spawni (pid_t *pid, const char *file,
       return __hurd_file_name_lookup (&child_init_port, &child_fd, 0,
 				      file, oflag, mode, result);
     }
+  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.  */
+
+      /* Append trailing "/." to directory name to force ENOTDIR if
+	 it's not a directory and EACCES if we don't have search
+	 permission.  */
+      len = strlen (name);
+      const char *lookup = name;
+      if (len >= 2 && name[len - 2] == '/' && name[len - 1] == '.')
+	lookup = name;
+      else if (len == 0)
+	/* Special-case empty file name according to POSIX.  */
+	return __hurd_fail (ENOENT);
+      else
+	{
+	  char *n = alloca (len + 3);
+	  memcpy (n, name, len);
+	  n[len] = '/';
+	  n[len + 1] = '.';
+	  n[len + 2] = '\0';
+	  lookup = n;
+	}
+
+      return child_lookup (lookup, 0, 0, &rcwdir);
+    }
 
 
   /* Do this once.  */
@@ -485,6 +517,10 @@ __spawni (pid_t *pid, const char *file,
 	      dtable_cells[fd] = NULL;
 	      break;
 	    }
+
+	  case spawn_do_chdir:
+	    err = child_chdir (action->action.chdir_action.path);
+	    break;
 	  }
 
 	if (err)

Re: [PATCH] Hurd: Implement chdir support in posix_spawn

[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.

No problem, I'll have a look and test :)

Thanks!
Samuel

Re: [PATCH] Hurd: Implement chdir support in posix_spawn

[Samuel Thibault]

[-- Attachment #1: Type: text/plain, Size: 4201 bytes --]

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)

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3804 bytes --]

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)

Re: [PATCH] Hurd: Implement chdir support in posix_spawn

[Florian Weimer]

* 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

Re: [PATCH] Hurd: Implement chdir support in posix_spawn

[Samuel Thibault]

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