execve: fix deadlock/crash
authorAlan Cox <alan@linux.intel.com>
Sun, 4 Dec 2016 14:31:54 +0000 (14:31 +0000)
committerAlan Cox <alan@linux.intel.com>
Sun, 4 Dec 2016 14:31:54 +0000 (14:31 +0000)
If a process sets files to be closed with O_CLOEXEC and those files are ones
that sleep during the close (eg a socket), and at the same time as it is
sleeping then on a little box with not many buffers you can easily deadlock
or run out of buffers.

Only allow one execve process at a time. As the disk I/O on a little 8bit
micro takes the full CPU time, and the only blocking case is the obscure
O_CLOEXEC one the performance impact of this change is pretty close to nil,
but it does mean we need to be careful not to let anything block for long
while closing.

For networking this will need a little bit of care and it also introduces
a bizarre corner case deadlock if you are using the userspace networking
daemon. You must ensure the daemon does not itself try to do an execve
while a socket might be waiting to close. As the network daemon can already
tie itself in knots if it attempts to use sockets, and is very special this
seems acceptable.

Kernel/syscall_exec16.c

index db17de2..3ff1720 100644 (file)
@@ -86,6 +86,8 @@ static int header_ok(uint8_t *pp)
        return 1;
 }
 
+static uint8_t in_execve;
+
 arg_t _execve(void)
 {
        staticfast inoptr ino;
@@ -150,6 +152,13 @@ arg_t _execve(void)
                goto nogood2;
        }
 
+       /* We can't allow multiple execs to occur beyond this point at once
+          otherwise we may deadlock out of buffers. As we already assume
+          synchronous block I/O on 8bit boxes this isn't really a hit at all */
+       while(in_execve)
+               psleep(&in_execve);
+       in_execve = 1;
+
        /* Gather the arguments, and put them in temporary buffers. */
        abuf = (struct s_argblk *) tmpbuf();
        /* Put environment in another buffer. */
@@ -184,7 +193,7 @@ arg_t _execve(void)
        brelse(buf);
 
        /* At this point, we are committed to reading in and
-        * executing the program. */
+        * executing the program. This call can block. */
 
        close_on_exec();
 
@@ -242,12 +251,16 @@ arg_t _execve(void)
        udata.u_isp = nenvp - 2;
 
        // Start execution (never returns)
+       in_execve = 0;
+       wakeup(&in_execve);
        doexec(PROGLOAD);
 
        // tidy up in various failure modes:
 nogood3:
        brelse(abuf);
        brelse(ebuf);
+       in_execve = 0;
+       wakeup(&in_execve);
       nogood2:
        brelse(buf);
       nogood: