Date: Sun, 27 Oct 1996 12:14:41 +1100 From: Bruce Evans <bde@zeta.org.au> To: mark@grondar.za, sos@freefall.freebsd.org Cc: cvs-all@freefall.freebsd.org, CVS-committers@freefall.freebsd.org, cvs-sys@freefall.freebsd.org Subject: Re: cvs commit: src/sys/i386/isa syscons.c Message-ID: <199610270114.MAA06472@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>Soren Schmidt wrote: >> Fixed side effects from calling add_keyboard_randomness() in the console >> getc routine by not calling it. add_keyboard_randomness() currently >> always reenables interrupts on 386's and 486's. This is very bad if the >> console getc routine is called from the debugger and the debugger was >> entered with interrupts disabled. > >add_keyboard_randomness() calls add_timer_randomness(), which does this: > > disable_intr(); > outb(TIMER_MODE, TIMER_SEL0 | TIMER_LATCH); > num ^= inb(TIMER_CNTR0) << 16; > num ^= inb(TIMER_CNTR0) << 24; > enable_intr(); > >If this was changed to not disable/enable the interrupts, would it cause >any trouble? I don't mind getting funny numbers out of the counter, that >just adds to entropy, but causing the counter device to misbehave would be >a bad thing. It depends on what happens when the timer mode is set in interrupt handlers. If setting the mode clears the previous setting, then there is no problem. It could use read_eflags(); disable_intr(); ... write_eflags(); so that it always work when it is called with interrupts disabled, but it shouldn't be called with interrupts disabled. >If this was fixed, could this be added back to syscons? No. Console i/o routines should have as few side effects as possible, because they might be being used to debug things that would be corrupted by the side effects. There seems to be a more serious problem with reentrancy in add_timer_randomness(). It is called from interrupt handlers at the ipls of the handlers, so it may be reentered. It doesn't seem to be designed for this. I guess it works in Linux because it is always called with interrupts disabled. It should use splhigh() in FreeBSD. (Don't forget the design goal that interrupts should not be masked for too long. 20 usec is too long.) Since it is normally (*) called from the keyboard interrupt handler at ipl >= tty_imask, it is now only safe to gather entropy for interrupts generated by tty-class devices. (*) It is missing from pcvt_kbd.o in GENERIC and LINT, because PCVT_FREEBSD is still defined as 210 although 2.1.0R is ancient history. I hate ifdefs. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199610270114.MAA06472>