Date: Wed, 27 Jan 1999 23:35:01 +1100 From: Bruce Evans <bde@zeta.org.au> To: security@FreeBSD.ORG Subject: Re: signal handling in urandom can cause lockup Message-ID: <199901271235.XAA19607@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
I finally "finished" my fix for this.
The problem has very little to do with signal handling, at least under
FreeBSD. It is a general problem with slow devices that can transfer
large amounts of data without blocking. E.g.,
dd if=/dev/urandom of=/dev/null bs=10m count=1
runs at about 1MB per 3.8 seconds on a P5/133, so the read() syscall
for 10MB spends about 38 seconds in the kernel without blocking.
My fix blocks most i/o hog processes in uiomove() and associated
functions if they would be rescheduled if they were running in user
mode. Unfortunately, signals can't be handled at this level since
it would be surprising if disk i/o could be aborted by a signal.
My fix only checks for signals for /dev/urandom. I don't know of
any other devices that need it.
Bruce
diff -c2 src/sys/alpha/include/cpu.h~ src/sys/alpha/include/cpu.h
*** src/sys/alpha/include/cpu.h~ Wed Oct 7 21:32:44 1998
--- src/sys/alpha/include/cpu.h Tue Jan 26 00:14:30 1999
***************
*** 76,80 ****
* or after the current trap/syscall if in system mode.
*/
! #define need_resched() { want_resched = 1; aston(); }
/*
--- 76,82 ----
* or after the current trap/syscall if in system mode.
*/
! #define need_resched() do { want_resched = 1; aston(); } while (0)
!
! #define resched_wanted() want_resched
/*
diff -c2 src/sys/i386/include/cpu.h~ src/sys/i386/include/cpu.h
*** src/sys/i386/include/cpu.h~ Tue Sep 1 15:54:52 1998
--- src/sys/i386/include/cpu.h Tue Jan 26 00:13:47 1999
***************
*** 84,88 ****
* or after the current trap/syscall if in system mode.
*/
! #define need_resched() { want_resched = 1; aston(); }
/*
--- 84,90 ----
* or after the current trap/syscall if in system mode.
*/
! #define need_resched() do { want_resched = 1; aston(); } while (0)
!
! #define resched_wanted() want_resched
/*
diff -c2 src/sys/kern/kern_subr.c~ src/sys/kern/kern_subr.c
*** src/sys/kern/kern_subr.c~ Mon Jan 11 03:15:05 1999
--- src/sys/kern/kern_subr.c Tue Jan 26 00:41:50 1999
***************
*** 45,48 ****
--- 45,49 ----
#include <sys/malloc.h>
#include <sys/lock.h>
+ #include <sys/resourcevar.h>
#include <sys/vnode.h>
***************
*** 52,55 ****
--- 53,60 ----
#include <vm/vm_map.h>
+ #include <machine/cpu.h>
+
+ static int uio_yield __P((void));
+
int
uiomove(cp, n, uio)
***************
*** 82,85 ****
--- 87,92 ----
case UIO_USERSPACE:
case UIO_USERISPACE:
+ if (resched_wanted())
+ uio_yield();
if (uio->uio_rw == UIO_READ)
error = copyout(cp, iov->iov_base, cnt);
***************
*** 140,143 ****
--- 147,152 ----
case UIO_USERSPACE:
case UIO_USERISPACE:
+ if (resched_wanted())
+ uio_yield();
if (uio->uio_rw == UIO_READ) {
if (vfs_ioopt && ((cnt & PAGE_MASK) == 0) &&
***************
*** 215,218 ****
--- 224,229 ----
cnt &= ~PAGE_MASK;
+ if (resched_wanted())
+ uio_yield();
error = vm_uiomove(&curproc->p_vmspace->vm_map, obj,
uio->uio_offset, cnt,
***************
*** 389,391 ****
--- 400,417 ----
*nentries = hashsize;
return (hashtbl);
+ }
+
+ static int
+ uio_yield()
+ {
+ struct proc *p;
+ int s;
+
+ p = curproc;
+ s = splhigh();
+ setrunqueue(p);
+ p->p_stats->p_ru.ru_nivcsw++;
+ mi_switch();
+ splx(s);
+ return (0);
}
diff -c2 src/sys/i386/i386/mem.c~ src/sys/i386/i386/mem.c
*** src/sys/i386/i386/mem.c~ Mon Nov 9 16:34:49 1998
--- src/sys/i386/i386/mem.c Wed Jan 27 23:04:33 1999
***************
*** 60,63 ****
--- 60,64 ----
#include <sys/malloc.h>
#include <sys/proc.h>
+ #include <sys/signalvar.h>
#include <machine/frame.h>
***************
*** 287,290 ****
--- 288,301 ----
c = iov->iov_len;
break;
+ }
+ if (CURSIG(curproc) != 0) {
+ /*
+ * Use tsleep() to get the error code right.
+ * It should return immediately.
+ */
+ error = tsleep(&random_softc[0],
+ PZERO | PCATCH, "urand", 1);
+ if (error != 0 && error != EWOULDBLOCK)
+ continue;
}
if (buf == NULL)
>From owner-freebsd-bugs@FreeBSD.ORG Thu Dec 31 13:12:07 1998
>Return-Path: <owner-freebsd-bugs@FreeBSD.ORG>
>Received: from gidora.zeta.org.au (gidora.zeta.org.au [203.26.10.25])
> by godzilla.zeta.org.au (8.8.7/8.8.7) with SMTP id NAA23413
> for <bde@zeta.org.au>; Thu, 31 Dec 1998 13:12:06 +1100
>Received: (qmail 7350 invoked from network); 31 Dec 1998 02:12:04 -0000
>Received: from hub.freebsd.org (204.216.27.18)
> by gidora.zeta.org.au with SMTP; 31 Dec 1998 02:12:04 -0000
>Received: from localhost (daemon@localhost)
> by hub.freebsd.org (8.8.8/8.8.8) with SMTP id RAA02579;
> Wed, 30 Dec 1998 17:56:31 -0800 (PST)
> (envelope-from owner-freebsd-bugs)
>Received: by hub.freebsd.org (bulk_mailer v1.6); Wed, 30 Dec 1998 17:56:17 -0800
>Received: (from majordom@localhost)
> by hub.freebsd.org (8.8.8/8.8.8) id RAA02486
> for freebsd-bugs-outgoing; Wed, 30 Dec 1998 17:56:17 -0800 (PST)
> (envelope-from owner-freebsd-bugs@FreeBSD.ORG)
>Received: from ouch.Oof.NET (ouch.Oof.NET [208.212.72.34])
> by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id RAA02481
> for <freebsd-bugs@FreeBSD.ORG>; Wed, 30 Dec 1998 17:56:13 -0800 (PST)
> (envelope-from sdjames@research.poc.net)
>Received: from localhost (sdj@localhost)
> by ouch.Oof.NET (POC/Oof) with ESMTP id UAA17346;
> Wed, 30 Dec 1998 20:55:14 -0500 (EST)
>Date: Wed, 30 Dec 1998 20:55:14 -0500 (EST)
>From: Steven Ji <sdjames@research.poc.net>
>To: freebsd-bugs@FreeBSD.ORG
>Subject: signal handling in urandom can cause lockup
>Message-ID: <Pine.BSF.4.05.9812302053180.17259-100000@ouch.Oof.NET>
>MIME-Version: 1.0
>Content-Type: TEXT/PLAIN; charset=US-ASCII
>Sender: owner-freebsd-bugs@FreeBSD.ORG
>X-Loop: FreeBSD.org
>Status: RO
>
>dd if=/dev/urandom of=/dev/null bs=100000k count=20000
>
>I just tried this on -stable (12/2/98), and the machine seemed to
>completely lock up.
>
>Box is a P6@200MHz x 2.
>
>---------- Forwarded message ----------
>Date: Sun, 27 Dec 1998 20:40:32 +0100
>From: Andrea Arcangeli <andrea@E-MIND.COM>
>To: BUGTRAQ@netspace.org
>Subject: [patch] fix for urandom read(2) not interruptible
>
>After having read phrak54 about Linux /dev/u?random (and this is the reason
>I am CCing also to bugtraq ;), I was playing a bit
>with the random driver it and I noticed that was difficult to kill `dd
>if=/dev/urandom of=/dev/null bs=100000k count=20000' once started ;)). The
>machine was eavily loaded and the process was unkillable and I the fastest
>thing to restore the system is been a reset...
>
>It's a bug in random.c that doesn' t check for signal pending inside the
>read(2) code, so you have no chance to kill the process via signals until
>the read(2) syscall is finished, and it could take a lot of time before
>return, if the buffer given to the read syscall is very big...
>
>Here the fix against 2.1.132:
>
>Index: linux/drivers/char/random.c
>diff -u linux/drivers/char/random.c:1.1.1.1 linux/drivers/char/random.c:1.1.1.1.2.3
>--- linux/drivers/char/random.c:1.1.1.1 Fri Nov 20 00:02:25 1998
>+++ linux/drivers/char/random.c Sun Dec 27 20:19:16 1998
>@@ -232,6 +232,11 @@
> * Eastlake, Steve Crocker, and Jeff Schiller.
> */
>
>+/*
>+ * Added a check for signal pending in the extract_entropy() loop to allow
>+ * the read(2) syscall to be interrupted. Copyright (C) 1998 Andrea Arcangeli
>+ */
>+
> #include <linux/utsname.h>
> #include <linux/config.h>
> #include <linux/kernel.h>
>@@ -1269,7 +1274,14 @@
> buf += i;
> add_timer_randomness(r, &extract_timer_state, nbytes);
> if (to_user && current->need_resched)
>+ {
>+ if (signal_pending(current))
>+ {
>+ ret = -EINTR;
>+ break;
>+ }
> schedule();
>+ }
> }
>
> /* Wipe data just returned from memory */
>
>
>And here a fix against 2.0.36:
>
>--- linux/drivers/char/random.c.orig Sun Dec 27 20:22:53 1998
>+++ linux/drivers/char/random.c Sun Dec 27 20:24:17 1998
>@@ -226,6 +226,11 @@
> * Eastlake, Steve Crocker, and Jeff Schiller.
> */
>
>+/*
>+ * Added a check for signal pending in the extract_entropy() loop to allow
>+ * the read(2) syscall to be interrupted. Copyright (C) 1998 Andrea Arcangeli
>+ */
>+
> #include <linux/config.h> /* CONFIG_RST_COOKIES and CONFIG_SYN_COOKIES */
> #include <linux/utsname.h>
> #include <linux/kernel.h>
>@@ -1004,7 +1009,14 @@
> buf += i;
> add_timer_randomness(r, &extract_timer_state, nbytes);
> if (to_user && need_resched)
>+ {
>+ if (signal_pending(current))
>+ {
>+ ret = -EINTR;
>+ break;
>+ }
> schedule();
>+ }
> }
>
> /* Wipe data from memory */
>
>Andrea Arcangeli
>
>
>To Unsubscribe: send mail to majordomo@FreeBSD.org
>with "unsubscribe freebsd-bugs" in the body of the message
>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-security" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199901271235.XAA19607>