[PATCH 0/1] MR6790: ntdll: use fork()/exec() to invoke unmount command instead of system().

[Michael Lelli (＠toadking)]

Fix umounting filesystems mounted on paths with spaces.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57391

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6790

[PATCH 1/1] ntdll: use fork()/exec() to invoke unmount command instead of system().

[Michael Lelli]

From: Michael Lelli <toadking(a)toadking.com>

Fix umounting filesystems mounted on paths with spaces.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57391
---
 dlls/ntdll/unix/file.c | 71 +++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 15 deletions(-)

diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index 44adc4f4626..3f478bcd35b 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -52,6 +52,7 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 #include <sys/ioctl.h>
+#include <sys/wait.h>
 #ifdef HAVE_SYS_ATTR_H
 #include <sys/attr.h>
 #endif
@@ -3954,6 +3955,56 @@ NTSTATUS get_full_path( const WCHAR *name, const WCHAR *curdir, WCHAR **path )
 }
 
 
+/***********************************************************************
+ *           run_unmount_command
+ *
+ * Run the platform-specific unmount command for the device.
+ */
+static NTSTATUS run_unmount_command( char *mount_point )
+{
+    NTSTATUS status;
+    pid_t pid;
+#ifdef __APPLE__
+    static char diskutil[] = "diskutil";
+    static char unmount[] = "unmount";
+    char *argv[4] = {diskutil, unmount, mount_point, NULL};
+#else
+    static char umount[] = "umount";
+    char *argv[3] = {umount, mount_point, NULL};
+#endif
+
+    if (!(pid = fork()))
+    {
+        /* in child */
+        int fd = open( "/dev/null", O_RDWR );
+        if (fd != -1)
+        {
+            dup2( fd, 0 );
+            dup2( fd, 1 );
+            dup2( fd, 2 );
+            close( fd );
+        }
+        execvp( argv[0], argv );
+
+        /* error running execvp */
+        _exit( 1 );
+    }
+
+    if (pid != -1)
+    {
+        /* reap child */
+        pid_t wret;
+        do {
+            wret = waitpid(pid, NULL, 0);
+        } while (wret < 0 && errno == EINTR);
+        status = STATUS_SUCCESS;
+    }
+    else status = STATUS_NO_MEMORY;
+
+    return status;
+}
+
+
 /***********************************************************************
  *           unmount_device
  *
@@ -3975,23 +4026,13 @@ static NTSTATUS unmount_device( HANDLE handle )
         {
             if ((mount_point = get_device_mount_point( st.st_rdev )))
             {
-#ifdef __APPLE__
-                static const char umount[] = "diskutil unmount >/dev/null 2>&1 ";
-#else
-                static const char umount[] = "umount >/dev/null 2>&1 ";
-#endif
-                char *cmd;
-                if (asprintf( &cmd, "%s%s", umount, mount_point ) != -1)
-                {
-                    system( cmd );
-                    free( cmd );
+                if ((status = run_unmount_command( mount_point ))) return status;
+
 #ifdef linux
-                    /* umount will fail to release the loop device since we still have
-                       a handle to it, so we release it here */
-                    if (major(st.st_rdev) == LOOP_MAJOR) ioctl( unix_fd, 0x4c01 /*LOOP_CLR_FD*/, 0 );
+                /* umount will fail to release the loop device since we still have
+                    a handle to it, so we release it here */
+                if (major(st.st_rdev) == LOOP_MAJOR) ioctl( unix_fd, 0x4c01 /*LOOP_CLR_FD*/, 0 );
 #endif
-                }
-                free( mount_point );
             }
         }
         if (needs_close) close( unix_fd );
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6790

[Brendan Shanks (＠bshanks)]

I think it would be preferable to use `posix_spawn` instead of `fork`/`exec`. Or, maybe `__wine_unix_spawnvp()` could be used?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6790#note_86882