From d8beed10620c9fd8e93655e3108bd9ad35985d48 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Sun, 4 Dec 2016 14:31:54 +0000 Subject: [PATCH] execve: fix deadlock/crash 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 | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Kernel/syscall_exec16.c b/Kernel/syscall_exec16.c index db17de2c..3ff1720e 100644 --- a/Kernel/syscall_exec16.c +++ b/Kernel/syscall_exec16.c @@ -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: -- 2.34.1