This patch introduces an issue for me an embedded PowerPC SoC using the
8250 driver.
The simple description of my issue is this: I'm using the serial port for
both a terminal and console. I run fdisk on a /dev/hda. Before this
patch I would get the prompt for fdisk immediately. After this patch I
have to hit return before the prompt is displayed.
I know that's not a lot of info, but just let me know what else you need
to help debug this.
I'm guessing something about the UARTs on the PowerPC maybe bit a little
non-standard.
thanks
- kumar
On Mer, 2006-02-01 at 19:21 -0600, Kumar Gala wrote:
> This patch introduces an issue for me an embedded PowerPC SoC using the
> 8250 driver.
>
> The simple description of my issue is this: I'm using the serial port for
> both a terminal and console. I run fdisk on a /dev/hda. Before this
> patch I would get the prompt for fdisk immediately. After this patch I
> have to hit return before the prompt is displayed.
>
> I know that's not a lot of info, but just let me know what else you need
> to help debug this.
>
> I'm guessing something about the UARTs on the PowerPC maybe bit a little
> non-standard.
I wonder if I've swapped one race for another. Can you revert just the
line which forces THRI on and test with the rest of the change please.
On Feb 1, 2006, at 7:47 PM, Alan Cox wrote:
> On Mer, 2006-02-01 at 19:21 -0600, Kumar Gala wrote:
>> This patch introduces an issue for me an embedded PowerPC SoC
>> using the
>> 8250 driver.
>>
>> The simple description of my issue is this: I'm using the serial
>> port for
>> both a terminal and console. I run fdisk on a /dev/hda. Before this
>> patch I would get the prompt for fdisk immediately. After this
>> patch I
>> have to hit return before the prompt is displayed.
>>
>> I know that's not a lot of info, but just let me know what else
>> you need
>> to help debug this.
>>
>> I'm guessing something about the UARTs on the PowerPC maybe bit a
>> little
>> non-standard.
>
> I wonder if I've swapped one race for another. Can you revert just the
> line which forces THRI on and test with the rest of the change please.
This doesn't seem to help.
- kumar
On Feb 1, 2006, at 11:54 PM, Kumar Gala wrote:
>
> On Feb 1, 2006, at 7:47 PM, Alan Cox wrote:
>
>> On Mer, 2006-02-01 at 19:21 -0600, Kumar Gala wrote:
>>> This patch introduces an issue for me an embedded PowerPC SoC
>>> using the
>>> 8250 driver.
>>>
>>> The simple description of my issue is this: I'm using the serial
>>> port for
>>> both a terminal and console. I run fdisk on a /dev/hda. Before
>>> this
>>> patch I would get the prompt for fdisk immediately. After this
>>> patch I
>>> have to hit return before the prompt is displayed.
>>>
>>> I know that's not a lot of info, but just let me know what else
>>> you need
>>> to help debug this.
>>>
>>> I'm guessing something about the UARTs on the PowerPC maybe bit a
>>> little
>>> non-standard.
>>
>> I wonder if I've swapped one race for another. Can you revert just
>> the
>> line which forces THRI on and test with the rest of the change
>> please.
>
> This doesn't seem to help.
I realized a bit later that the kernel I build with your suggested
change wasn't the one I was testing. After loading the proper kernel
this does make the issue I was seeing go away.
- kumar
On Feb 2, 2006, at 2:05 AM, Kumar Gala wrote:
>
> On Feb 1, 2006, at 11:54 PM, Kumar Gala wrote:
>
>>
>> On Feb 1, 2006, at 7:47 PM, Alan Cox wrote:
>>
>>> On Mer, 2006-02-01 at 19:21 -0600, Kumar Gala wrote:
>>>> This patch introduces an issue for me an embedded PowerPC SoC
>>>> using the
>>>> 8250 driver.
>>>>
>>>> The simple description of my issue is this: I'm using the
>>>> serial port for
>>>> both a terminal and console. I run fdisk on a /dev/hda. Before
>>>> this
>>>> patch I would get the prompt for fdisk immediately. After this
>>>> patch I
>>>> have to hit return before the prompt is displayed.
>>>>
>>>> I know that's not a lot of info, but just let me know what else
>>>> you need
>>>> to help debug this.
>>>>
>>>> I'm guessing something about the UARTs on the PowerPC maybe bit
>>>> a little
>>>> non-standard.
>>>
>>> I wonder if I've swapped one race for another. Can you revert
>>> just the
>>> line which forces THRI on and test with the rest of the change
>>> please.
>>
>> This doesn't seem to help.
>
> I realized a bit later that the kernel I build with your suggested
> change wasn't the one I was testing. After loading the proper
> kernel this does make the issue I was seeing go away.
After some further testing with this change, the majority of issues I
was seeing go away. However, able to lock the console up. If I
revert the wait_for_xmitr() to always use BOTH_EMPTY the lock up goes
away. (Obviously, this is effectively reverting the whole patch).
- kumar
Hi Alan,
The serial console driver has a host of issues
- end of line should be CR LF, not LF CR
- 'r' option for CTS/RTS flow control assumes CTS is
asserted when DSR is not asserted. This is wrong as
lots of modems float control lines when off. No
control signals are defined is DSR is unasserted.
- [SECURITY] 'r' should require DCD to be asserted
before outputing characters. Otherwise we talk to
Hayes modem command mode. This allows a non-root
user to re-program the modem and is a major security
issue is people configure calling line identification
or encryption to restrict use of the serial console.
- 'r' option has insanely slow CTS timeout. So if a
terminal server is inactive the kernel can take
30 minutes to boot as each character write to the
serial console requires a CTS timeout.
All of these have been reported to me multiple times
(I'm maintainer of the Remote-Serial-Console-HOWTO).
I occassionally clean up and repost a patch I wrote years
ago which never gets integrated (although it ships in the
patchset of a number of kernels from supercomputer vendors).
I'm happy to clean it up again if there's a hope of
integration.
See
<http://www.aarnet.edu.au/~gdt/patch/console/serial-console.patch>
for the most recent attempt.
Thanks so much,
Glen
--
Glen Turner Tel: (08) 8303 3936 or +61 8 8303 3936
Australia's Academic & Research Network http://www.aarnet.edu.au
On Fri, Feb 03, 2006 at 12:28:46PM +1030, Glen Turner wrote:
> Hi Alan,
>
> The serial console driver has a host of issues
>
> [...]
>
> - [SECURITY] 'r' should require DCD to be asserted
> before outputing characters. Otherwise we talk to
> Hayes modem command mode. This allows a non-root
> user to re-program the modem and is a major security
> issue is people configure calling line identification
> or encryption to restrict use of the serial console.
How is this possible? A normal user can't produce arbitarily formatted
kernel messages, and if they have access to /dev/ttyS they can do what
ever they like with the port anyway.
(If a user can produce arbitarily formatted kernel messages, that in
itself is a security bug - how do you know if that OOPS was produced
by a malicious user, or a real oops?)
> - 'r' option has insanely slow CTS timeout. So if a
> terminal server is inactive the kernel can take
> 30 minutes to boot as each character write to the
> serial console requires a CTS timeout.
You'd rather we threw away these messages?
> I occassionally clean up and repost a patch I wrote years
> ago which never gets integrated (although it ships in the
> patchset of a number of kernels from supercomputer vendors).
> I'm happy to clean it up again if there's a hope of
> integration.
It'd help if you talked to the right person - I've been looking after
the serial layer since 2.5.something.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
>> - 'r' option has insanely slow CTS timeout. So if a
>> terminal server is inactive the kernel can take
>> 30 minutes to boot as each character write to the
>> serial console requires a CTS timeout.
>
> You'd rather we threw away these messages?
It seems to me that The Right Thing to do is, once I've given up on
CTS showing up and just sent the byte anyway, is send all future bytes
without waiting for CTS until CTS shows up again.
In other words, one timeout per falling edge of CTS.
This may be implementable neatly by just saying "fuck it; pretend CTS
is asserted" in the timeout handler if the later real assertion won't
confuse it.
Hi Russell,
Thanks for your response.
> A normal user can't produce arbitarily formatted
> kernel messages
They don't need to provide an entire message, just a
AT string (a vector which a user could control
could be a volume label on removable media).
> You'd rather we threw away these messages?
When option 'r' is set, yes. Because Data Carrier Detect
is not asserted there can't be anything that is going to
read the messages. All we are doing is taking a long
time to drop each character off the end of the serial
cable.
Since the kernel takes a very long time to boot with
the 'r' option and no attached console the current
situation isn't correct.
There's one very nasty practical effect. Hayes modems
hang up if any character is typed during the interval
between off-hook and Data Carrier Detect (ie, the negotiation
period where the speaker outputs the tones). If you're
continually writing console messages (say from a failing
disk) then you can't successfully dial in to read the
messages to find out why the machine is hosed.
> It'd help if you talked to the right person - I've been looking after
> the serial layer since 2.5.something.
See my e-mail to you of 2003-10-21, which started:
Hi Russell,
I have ported the patch which fixes all kernel bugs
reported to me as the maintainer of the "Remote Serial
Console HOWTO" from 2.4 to Linus's BitKeeper (2.6.0-test8).
and contained the patch at
<http://www.aarnet.edu.au/~gdt/patch/console/serial-console.patch>
The previous patch of 2002-10-13 gave a much longer description
of the bugs and fixes:
[PATCH] 0/3 Fix serial console flow control
<http://www.ussg.iu.edu/hypermail/linux/kernel/0210.1/1790.html>
[PATCH] 1/3 Fix serial console flow control, serial.c
<http://www.ussg.iu.edu/hypermail/linux/kernel/0210.1/1791.html>
As I said earlier I'm happy to once more forward-port the
patch, test it and present it using the current version control
system as long as the effort is worth doing.
Regards,
Glen
--
Glen Turner Tel: (08) 8303 3936 or +61 8 8303 3936
Australia's Academic & Research Network http://www.aarnet.edu.au
Russell,
Any theories on the initial issue that Alan's patch introduces for me
on my embedded PPC. At this point I have to revert the whole patch
to make this work "correctly".
thanks
- k
On Sat, Feb 04, 2006 at 12:57:28AM +1030, Glen Turner wrote:
>
> Hi Russell,
>
> Thanks for your response.
>
> > A normal user can't produce arbitarily formatted
> > kernel messages
>
> They don't need to provide an entire message, just a
> AT string (a vector which a user could control
> could be a volume label on removable media).
So?
My point stands - if the user can provide an arbitary string to printk,
they can fake any kernel message. That in itself is a security bug.
If there is an instance of that, then that's the real bug which would
need fixing.
Once those bugs have been fixed, your claimed bug is also magically
fixed.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King wrote:
> My point stands - if the user can provide an arbitary string to printk,
> they can fake any kernel message. That in itself is a security bug.
> If there is an instance of that, then that's the real bug which would
> need fixing.
>
> Once those bugs have been fixed, your claimed bug is also magically
> fixed.
Hi Russell,
Thanks for the explanation of where the kernel should handle
covert channels.
How about the other bugs reported by people who have used
the Remote-Serial-Console-HOWTO:
- writing any text to an idle (DCD not asserted) modem still
causes incoming calls to be hung up on. That's not good
as sysadmins can't connect to systems with failing hardware.
[Note that modems really need the 'r' option, so it's
fine to continue to write with DCD unasserted without
the 'r' option.]
- the huge boot times with the 'r' option and an idle/
unconnected modem/terminal server. This is caused by
the CTS timing out per character even when CTS is
floating (CTS is not defined unless DSR is asserted).
This basically makes the 'r' option impossible to
use on production systems. Not using the 'r' option
with a terminal server brings other problems (notably
character loss problems when people paste a large
number of characters into the SSH session through
the terminal server to the remote host).
- writing LF CR rather than CR LF unfortunately causes
issues with some terminals.
I'm really only the messenger here. I've collected bug reports
from readers of the HOWTO and written a patch to address their
experiences. I'm sure people with much more familiarity with
the kernel can do it better.
Thanks,
Glen
--
Glen Turner Tel: (08) 8303 3936 or +61 8 8303 3936
Australia's Academic & Research Network http://www.aarnet.edu.au
Russell King <[email protected]> writes:
> My point stands - if the user can provide an arbitary string to printk,
> they can fake any kernel message. That in itself is a security bug.
> If there is an instance of that, then that's the real bug which would
> need fixing.
I think the AT problem is valid - the user doesn't have to be able to
send anything to the console, the system alone can screw it up with
something as simple as ATZ^M (or with almost any string with embedded
[aA][tT].*^M).
_If_ we want to be able to connect a serial console to dial-up modem
we can't send _anything_ to it when DCD is down (except configuration
strings etc). I don't know exact timings but the modem will probably
accept commands hundreds of milliseconds after dropping DCD.
There are/were modems which could be configured using a jumper to
password-protected power-on dial-in mode and which doesn't accept
further Hayes commands - those are safe (they don't even abort
inbound connection negotiation if they receive characters from
the computer).
--
Krzysztof Halasa
On Fri, Feb 03, 2006 at 06:46:24PM +0100, Krzysztof Halasa wrote:
> Russell King <[email protected]> writes:
>
> > My point stands - if the user can provide an arbitary string to printk,
> > they can fake any kernel message. That in itself is a security bug.
> > If there is an instance of that, then that's the real bug which would
> > need fixing.
>
> I think the AT problem is valid - the user doesn't have to be able to
> send anything to the console, the system alone can screw it up with
> something as simple as ATZ^M (or with almost any string with embedded
> [aA][tT].*^M).
Stop throwing FUD into this issue. The original claim was that a
non-root user could send arbitary strings via the console system.
This was an independent claim from the other issues.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 04, 2006 at 03:38:16AM +1030, Glen Turner wrote:
> How about the other bugs reported by people who have used
> the Remote-Serial-Console-HOWTO:
>
> - writing any text to an idle (DCD not asserted) modem still
> causes incoming calls to be hung up on. That's not good
> as sysadmins can't connect to systems with failing hardware.
>
> [Note that modems really need the 'r' option, so it's
> fine to continue to write with DCD unasserted without
> the 'r' option.]
What about those who have incomplete null modem cables which might
not connect DCD or DSR, but who want to use hardware flow control?
> - the huge boot times with the 'r' option and an idle/
> unconnected modem/terminal server. This is caused by
> the CTS timing out per character even when CTS is
> floating (CTS is not defined unless DSR is asserted).
> This basically makes the 'r' option impossible to
> use on production systems.
Not convinced about this claim - see above.
> Not using the 'r' option
> with a terminal server brings other problems (notably
> character loss problems when people paste a large
> number of characters into the SSH session through
> the terminal server to the remote host).
If you're talking about the terminal server sending to the Linux console,
that's got absolutely nothing to do with the 'r' option. The 'r' option
only controls the settings for the kernel-side of the console, which is
strictly output only. The input side is handled just like any tty, and
the termios settings define how that behaves.
Hence, if you enable CRTSCTS without 'r' then you will have flow control
for the userspace side, and no flow control for the kernel messages.
Thinking about this more, all your issues can be cleared up quite
trivially IMHO. Consider 'r' to mean "I want my kernel console to
try to not drop any kernel messages" and the CRTSCTS termios flag
to mean "I want flow control for non-kernel input/output". Hence,
if you don't want the modem to be able to effectively halt the kernel,
but you do want reliable communications for getty/shell etc, don't
specify 'r' but ensure that CRTSCTS gets set.
> - writing LF CR rather than CR LF unfortunately causes
> issues with some terminals.
This one I agree with. The well known sequence is CRLF not LFCR.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Fri, Feb 03, 2006 at 10:23:40PM +0000, Russell King wrote:
> On Sat, Feb 04, 2006 at 03:38:16AM +1030, Glen Turner wrote:
> > - writing LF CR rather than CR LF unfortunately causes
> > issues with some terminals.
>
> This one I agree with. The well known sequence is CRLF not LFCR.
And here's a patch which fixes this particular issue across some of
the many drivers we have... and also should make further fixes more
trivial to propagate to all drivers.
There's a number of drivers which haven't been done, because they
use non-uart_port things in the console code. However, these could
probably be bent to use a uart_port instead and take advantage of
uart_console_write():
- ip22zilog.c
- sunzilog.c
- v850e_uart.c
These have different requirements from usual uarts:
- mpc52xx_uart.c
- mpsc.c
diff --git a/drivers/serial/21285.c b/drivers/serial/21285.c
--- a/drivers/serial/21285.c
+++ b/drivers/serial/21285.c
@@ -375,23 +375,18 @@ static void serial21285_setup_ports(void
}
#ifdef CONFIG_SERIAL_21285_CONSOLE
+static void serial21285_console_putchar(struct uart_port *port, char ch)
+{
+ while (*CSR_UARTFLG & 0x20)
+ barrier();
+ *CSR_UARTDR = ch;
+}
static void
serial21285_console_write(struct console *co, const char *s,
unsigned int count)
{
- int i;
-
- for (i = 0; i < count; i++) {
- while (*CSR_UARTFLG & 0x20)
- barrier();
- *CSR_UARTDR = s[i];
- if (s[i] == '\n') {
- while (*CSR_UARTFLG & 0x20)
- barrier();
- *CSR_UARTDR = '\r';
- }
- }
+ uart_console_write(&serial21285_port, s, count, serial21285_console_putchar);
}
static void __init
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2182,6 +2182,14 @@ static inline void wait_for_xmitr(struct
}
}
+static void serial8250_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_8250_port *up = (struct uart_8250_port *)port;
+
+ wait_for_xmitr(up, UART_LSR_THRE);
+ serial_out(up, UART_TX, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -2193,7 +2201,6 @@ serial8250_console_write(struct console
{
struct uart_8250_port *up = &serial8250_ports[co->index];
unsigned int ier;
- int i;
touch_nmi_watchdog();
@@ -2207,22 +2214,7 @@ serial8250_console_write(struct console
else
serial_out(up, UART_IER, 0);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++, s++) {
- wait_for_xmitr(up, UART_LSR_THRE);
-
- /*
- * Send the character out.
- * If a LF, also do CR...
- */
- serial_out(up, UART_TX, *s);
- if (*s == 10) {
- wait_for_xmitr(up, UART_LSR_THRE);
- serial_out(up, UART_TX, 13);
- }
- }
+ uart_console_write(&up->port, s, count, serial8250_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/8250_early.c b/drivers/serial/8250_early.c
--- a/drivers/serial/8250_early.c
+++ b/drivers/serial/8250_early.c
@@ -74,7 +74,7 @@ static void __init wait_for_xmitr(struct
}
}
-static void __init putc(struct uart_port *port, unsigned char c)
+static void __init putc(struct uart_port *port, char c)
{
wait_for_xmitr(port);
serial_out(port, UART_TX, c);
@@ -89,12 +89,7 @@ static void __init early_uart_write(stru
ier = serial_in(port, UART_IER);
serial_out(port, UART_IER, 0);
- while (*s && count-- > 0) {
- putc(port, *s);
- if (*s == '\n')
- putc(port, '\r');
- s++;
- }
+ uart_console_write(port, s, count, putc);
/* Wait for transmitter to become empty and restore the IER */
wait_for_xmitr(port);
diff --git a/drivers/serial/amba-pl010.c b/drivers/serial/amba-pl010.c
--- a/drivers/serial/amba-pl010.c
+++ b/drivers/serial/amba-pl010.c
@@ -591,12 +591,18 @@ static struct uart_amba_port amba_ports[
#ifdef CONFIG_SERIAL_AMBA_PL010_CONSOLE
+static void pl010_console_putchar(struct uart_port *port, char ch)
+{
+ while (!UART_TX_READY(UART_GET_FR(port)))
+ barrier();
+ UART_PUT_CHAR(port, ch);
+}
+
static void
pl010_console_write(struct console *co, const char *s, unsigned int count)
{
struct uart_port *port = &amba_ports[co->index].port;
unsigned int status, old_cr;
- int i;
/*
* First save the CR then disable the interrupts
@@ -604,21 +610,7 @@ pl010_console_write(struct console *co,
old_cr = UART_GET_CR(port);
UART_PUT_CR(port, UART01x_CR_UARTEN);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++) {
- do {
- status = UART_GET_FR(port);
- } while (!UART_TX_READY(status));
- UART_PUT_CHAR(port, s[i]);
- if (s[i] == '\n') {
- do {
- status = UART_GET_FR(port);
- } while (!UART_TX_READY(status));
- UART_PUT_CHAR(port, '\r');
- }
- }
+ uart_console_write(port, s, count, pl010_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/amba-pl011.c b/drivers/serial/amba-pl011.c
--- a/drivers/serial/amba-pl011.c
+++ b/drivers/serial/amba-pl011.c
@@ -587,14 +587,12 @@ static struct uart_amba_port *amba_ports
#ifdef CONFIG_SERIAL_AMBA_PL011_CONSOLE
-static inline void
-pl011_console_write_char(struct uart_amba_port *uap, char ch)
+static void pl011_console_putchar(struct uart_port *port, char ch)
{
- unsigned int status;
+ struct uart_amba_port *uap = (struct uart_amba_port *)port;
- do {
- status = readw(uap->port.membase + UART01x_FR);
- } while (status & UART01x_FR_TXFF);
+ while (readw(uap->port.membase + UART01x_FR) & UART01x_FR_TXFF)
+ barrier();
writew(ch, uap->port.membase + UART01x_DR);
}
@@ -603,7 +601,6 @@ pl011_console_write(struct console *co,
{
struct uart_amba_port *uap = amba_ports[co->index];
unsigned int status, old_cr, new_cr;
- int i;
clk_enable(uap->clk);
@@ -615,14 +612,7 @@ pl011_console_write(struct console *co,
new_cr |= UART01x_CR_UARTEN | UART011_CR_TXE;
writew(new_cr, uap->port.membase + UART011_CR);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++) {
- pl011_console_write_char(uap, s[i]);
- if (s[i] == '\n')
- pl011_console_write_char(uap, '\r');
- }
+ uart_console_write(&uap->port, s, count, pl011_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/at91_serial.c b/drivers/serial/at91_serial.c
--- a/drivers/serial/at91_serial.c
+++ b/drivers/serial/at91_serial.c
@@ -711,6 +711,12 @@ void __init at91_register_uart(int idx,
}
#ifdef CONFIG_SERIAL_AT91_CONSOLE
+static void at91_console_putchar(struct uart_port *port, char ch)
+{
+ while (!(UART_GET_CSR(port) & AT91_US_TXRDY))
+ barrier();
+ UART_PUT_CHAR(port, ch);
+}
/*
* Interrupts are disabled on entering
@@ -718,7 +724,7 @@ void __init at91_register_uart(int idx,
static void at91_console_write(struct console *co, const char *s, u_int count)
{
struct uart_port *port = at91_ports + co->index;
- unsigned int status, i, imr;
+ unsigned int status, imr;
/*
* First, save IMR and then disable interrupts
@@ -726,21 +732,7 @@ static void at91_console_write(struct co
imr = UART_GET_IMR(port); /* get interrupt mask */
UART_PUT_IDR(port, AT91_US_RXRDY | AT91_US_TXRDY);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++) {
- do {
- status = UART_GET_CSR(port);
- } while (!(status & AT91_US_TXRDY));
- UART_PUT_CHAR(port, s[i]);
- if (s[i] == '\n') {
- do {
- status = UART_GET_CSR(port);
- } while (!(status & AT91_US_TXRDY));
- UART_PUT_CHAR(port, '\r');
- }
- }
+ uart_console_write(port, s, count, at91_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/au1x00_uart.c b/drivers/serial/au1x00_uart.c
--- a/drivers/serial/au1x00_uart.c
+++ b/drivers/serial/au1x00_uart.c
@@ -1121,6 +1121,14 @@ static inline void wait_for_xmitr(struct
}
}
+static void au1x00_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_8250_port *up = (struct uart_8250_port *)port;
+
+ wait_for_xmitr(up);
+ serial_out(up, UART_TX, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -1132,7 +1140,6 @@ serial8250_console_write(struct console
{
struct uart_8250_port *up = &serial8250_ports[co->index];
unsigned int ier;
- int i;
/*
* First save the UER then disable the interrupts
@@ -1140,22 +1147,7 @@ serial8250_console_write(struct console
ier = serial_in(up, UART_IER);
serial_out(up, UART_IER, 0);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++, s++) {
- wait_for_xmitr(up);
-
- /*
- * Send the character out.
- * If a LF, also do CR...
- */
- serial_out(up, UART_TX, *s);
- if (*s == 10) {
- wait_for_xmitr(up);
- serial_out(up, UART_TX, 13);
- }
- }
+ uart_console_write(&up->port, s, count, au1x00_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/clps711x.c b/drivers/serial/clps711x.c
--- a/drivers/serial/clps711x.c
+++ b/drivers/serial/clps711x.c
@@ -424,6 +424,13 @@ static struct uart_port clps711x_ports[U
};
#ifdef CONFIG_SERIAL_CLPS711X_CONSOLE
+static void clps711xuart_console_putchar(struct uart_port *port, char ch)
+{
+ while (clps_readl(SYSFLG(port)) & SYSFLG_UTXFF)
+ barrier();
+ clps_writel(ch, UARTDR(port));
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -438,7 +445,6 @@ clps711xuart_console_write(struct consol
{
struct uart_port *port = clps711x_ports + co->index;
unsigned int status, syscon;
- int i;
/*
* Ensure that the port is enabled.
@@ -446,21 +452,7 @@ clps711xuart_console_write(struct consol
syscon = clps_readl(SYSCON(port));
clps_writel(syscon | SYSCON_UARTEN, SYSCON(port));
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++) {
- do {
- status = clps_readl(SYSFLG(port));
- } while (status & SYSFLG_UTXFF);
- clps_writel(s[i], UARTDR(port));
- if (s[i] == '\n') {
- do {
- status = clps_readl(SYSFLG(port));
- } while (status & SYSFLG_UTXFF);
- clps_writel('\r', UARTDR(port));
- }
- }
+ uart_console_write(port, s, count, clps711xuart_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/dz.c b/drivers/serial/dz.c
--- a/drivers/serial/dz.c
+++ b/drivers/serial/dz.c
@@ -673,11 +673,12 @@ static void dz_reset(struct dz_port *dpo
}
#ifdef CONFIG_SERIAL_DZ_CONSOLE
-static void dz_console_put_char(struct dz_port *dport, unsigned char ch)
+static void dz_console_putchar(struct uart_port *port, char ch)
{
+ struct dz_port *dport = (struct dz_port *)uport;
unsigned long flags;
int loops = 2500;
- unsigned short tmp = ch;
+ unsigned short tmp = (unsigned char)ch;
/* this code sends stuff out to serial device - spinning its
wheels and waiting. */
@@ -693,6 +694,7 @@ static void dz_console_put_char(struct d
spin_unlock_irqrestore(&dport->port.lock, flags);
}
+
/*
* -------------------------------------------------------------------
* dz_console_print ()
@@ -709,11 +711,7 @@ static void dz_console_print(struct cons
#ifdef DEBUG_DZ
prom_printf((char *) str);
#endif
- while (count--) {
- if (*str == '\n')
- dz_console_put_char(dport, '\r');
- dz_console_put_char(dport, *str++);
- }
+ uart_console_write(&dport->port, str, count, dz_console_putchar);
}
static int __init dz_console_setup(struct console *co, char *options)
diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
--- a/drivers/serial/imx.c
+++ b/drivers/serial/imx.c
@@ -743,6 +743,13 @@ static void __init imx_init_ports(void)
}
#ifdef CONFIG_SERIAL_IMX_CONSOLE
+static void imx_console_putchar(struct uart_port *port, char ch)
+{
+ struct imx_port *sport = (struct imx_port *)port;
+ while ((UTS((u32)sport->port.membase) & UTS_TXFULL))
+ barrier();
+ URTX0((u32)sport->port.membase) = ch;
+}
/*
* Interrupts are disabled on entering
@@ -751,7 +758,7 @@ static void
imx_console_write(struct console *co, const char *s, unsigned int count)
{
struct imx_port *sport = &imx_ports[co->index];
- unsigned int old_ucr1, old_ucr2, i;
+ unsigned int old_ucr1, old_ucr2;
/*
* First, save UCR1/2 and then disable interrupts
@@ -764,22 +771,7 @@ imx_console_write(struct console *co, co
& ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN);
UCR2((u32)sport->port.membase) = old_ucr2 | UCR2_TXEN;
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++) {
-
- while ((UTS((u32)sport->port.membase) & UTS_TXFULL))
- barrier();
-
- URTX0((u32)sport->port.membase) = s[i];
-
- if (s[i] == '\n') {
- while ((UTS((u32)sport->port.membase) & UTS_TXFULL))
- barrier();
- URTX0((u32)sport->port.membase) = '\r';
- }
- }
+ uart_console_write(&sport->port, s, count, imx_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/ip22zilog.c b/drivers/serial/ip22zilog.c
--- a/drivers/serial/ip22zilog.c
+++ b/drivers/serial/ip22zilog.c
@@ -1000,9 +1000,9 @@ ip22zilog_console_write(struct console *
spin_lock_irqsave(&up->port.lock, flags);
for (i = 0; i < count; i++, s++) {
+ if (*s == '\n')
+ ip22zilog_put_char(channel, '\r');
ip22zilog_put_char(channel, *s);
- if (*s == 10)
- ip22zilog_put_char(channel, 13);
}
udelay(2);
spin_unlock_irqrestore(&up->port.lock, flags);
diff --git a/drivers/serial/m32r_sio.c b/drivers/serial/m32r_sio.c
--- a/drivers/serial/m32r_sio.c
+++ b/drivers/serial/m32r_sio.c
@@ -1039,6 +1039,14 @@ static inline void wait_for_xmitr(struct
}
}
+static void m32r_sio_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_sio_port *up = (struct uart_sio_port *)port;
+
+ wait_for_xmitr(up);
+ sio_out(up, SIOTXB, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -1058,23 +1066,7 @@ static void m32r_sio_console_write(struc
ier = sio_in(up, SIOTRCR);
sio_out(up, SIOTRCR, 0);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++, s++) {
- wait_for_xmitr(up);
-
- /*
- * Send the character out.
- * If a LF, also do CR...
- */
- sio_out(up, SIOTXB, *s);
-
- if (*s == 10) {
- wait_for_xmitr(up);
- sio_out(up, SIOTXB, 13);
- }
- }
+ uart_console_write(&up->port, s, count, m32r_sio_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -603,15 +603,14 @@ mpc52xx_console_write(struct console *co
udelay(1);
/* Write all the chars */
- for ( i=0 ; i<count ; i++ ) {
-
- /* Send the char */
- out_8(&psc->mpc52xx_psc_buffer_8, *s);
-
+ for (i = 0; i < count; i++, s++) {
/* Line return handling */
- if ( *s++ == '\n' )
+ if (*s == '\n')
out_8(&psc->mpc52xx_psc_buffer_8, '\r');
+ /* Send the char */
+ out_8(&psc->mpc52xx_psc_buffer_8, *s);
+
/* Wait the TX buffer to be empty */
j = 20000; /* Maximum wait */
while (!(in_be16(&psc->mpc52xx_psc_status) &
diff --git a/drivers/serial/pmac_zilog.c b/drivers/serial/pmac_zilog.c
--- a/drivers/serial/pmac_zilog.c
+++ b/drivers/serial/pmac_zilog.c
@@ -1916,6 +1916,16 @@ static void __exit exit_pmz(void)
#ifdef CONFIG_SERIAL_PMACZILOG_CONSOLE
+static void pmz_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_pmac_port *uap = (struct uart_pmac_port *)port;
+
+ /* Wait for the transmit buffer to empty. */
+ while ((read_zsreg(uap, R0) & Tx_BUF_EMP) == 0)
+ udelay(5);
+ write_zsdata(uap, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -1924,7 +1934,6 @@ static void pmz_console_write(struct con
{
struct uart_pmac_port *uap = &pmz_ports[con->index];
unsigned long flags;
- int i;
if (ZS_IS_ASLEEP(uap))
return;
@@ -1934,17 +1943,7 @@ static void pmz_console_write(struct con
write_zsreg(uap, R1, uap->curregs[1] & ~TxINT_ENAB);
write_zsreg(uap, R5, uap->curregs[5] | TxENABLE | RTS | DTR);
- for (i = 0; i < count; i++) {
- /* Wait for the transmit buffer to empty. */
- while ((read_zsreg(uap, R0) & Tx_BUF_EMP) == 0)
- udelay(5);
- write_zsdata(uap, s[i]);
- if (s[i] == 10) {
- while ((read_zsreg(uap, R0) & Tx_BUF_EMP) == 0)
- udelay(5);
- write_zsdata(uap, R13);
- }
- }
+ uart_console_write(&uap->port, s, count, pmz_console_putchar);
/* Restore the values in the registers. */
write_zsreg(uap, R1, uap->curregs[1]);
diff --git a/drivers/serial/pxa.c b/drivers/serial/pxa.c
--- a/drivers/serial/pxa.c
+++ b/drivers/serial/pxa.c
@@ -619,6 +619,14 @@ static inline void wait_for_xmitr(struct
}
}
+static void serial_pxa_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_pxa_port *up = (struct uart_pxa_port *)port;
+
+ wait_for_xmitr(up);
+ serial_out(up, UART_TX, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -630,7 +638,6 @@ serial_pxa_console_write(struct console
{
struct uart_pxa_port *up = &serial_pxa_ports[co->index];
unsigned int ier;
- int i;
/*
* First save the IER then disable the interrupts
@@ -638,22 +645,7 @@ serial_pxa_console_write(struct console
ier = serial_in(up, UART_IER);
serial_out(up, UART_IER, UART_IER_UUE);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++, s++) {
- wait_for_xmitr(up);
-
- /*
- * Send the character out.
- * If a LF, also do CR...
- */
- serial_out(up, UART_TX, *s);
- if (*s == 10) {
- wait_for_xmitr(up);
- serial_out(up, UART_TX, 13);
- }
- }
+ uart_console_write(&up->port, s, count, serial_pxa_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/s3c2410.c b/drivers/serial/s3c2410.c
--- a/drivers/serial/s3c2410.c
+++ b/drivers/serial/s3c2410.c
@@ -1580,25 +1580,19 @@ s3c24xx_serial_console_txrdy(struct uart
}
static void
-s3c24xx_serial_console_write(struct console *co, const char *s,
- unsigned int count)
+s3c24xx_serial_console_putchar(struct uart_port *port, char ch)
{
- int i;
unsigned int ufcon = rd_regl(cons_uart, S3C2410_UFCON);
+ while (!s3c24xx_serial_console_txrdy(port, ufcon))
+ barrier();
+ wr_regb(cons_uart, S3C2410_UTXH, ch);
+}
- for (i = 0; i < count; i++) {
- while (!s3c24xx_serial_console_txrdy(cons_uart, ufcon))
- barrier();
-
- wr_regb(cons_uart, S3C2410_UTXH, s[i]);
-
- if (s[i] == '\n') {
- while (!s3c24xx_serial_console_txrdy(cons_uart, ufcon))
- barrier();
-
- wr_regb(cons_uart, S3C2410_UTXH, '\r');
- }
- }
+static void
+s3c24xx_serial_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar);
}
static void __init
diff --git a/drivers/serial/sa1100.c b/drivers/serial/sa1100.c
--- a/drivers/serial/sa1100.c
+++ b/drivers/serial/sa1100.c
@@ -689,6 +689,14 @@ void __init sa1100_register_uart(int idx
#ifdef CONFIG_SERIAL_SA1100_CONSOLE
+static void sa1100_console_putchar(struct uart_port *port, char ch)
+{
+ struct sa1100_port *sport = (struct sa1100_port *)port;
+
+ while (!(UART_GET_UTSR1(sport) & UTSR1_TNF))
+ barrier();
+ UART_PUT_CHAR(sport, ch);
+}
/*
* Interrupts are disabled on entering
@@ -697,7 +705,7 @@ static void
sa1100_console_write(struct console *co, const char *s, unsigned int count)
{
struct sa1100_port *sport = &sa1100_ports[co->index];
- unsigned int old_utcr3, status, i;
+ unsigned int old_utcr3, status;
/*
* First, save UTCR3 and then disable interrupts
@@ -706,21 +714,7 @@ sa1100_console_write(struct console *co,
UART_PUT_UTCR3(sport, (old_utcr3 & ~(UTCR3_RIE | UTCR3_TIE)) |
UTCR3_TXE);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++) {
- do {
- status = UART_GET_UTSR1(sport);
- } while (!(status & UTSR1_TNF));
- UART_PUT_CHAR(sport, s[i]);
- if (s[i] == '\n') {
- do {
- status = UART_GET_UTSR1(sport);
- } while (!(status & UTSR1_TNF));
- UART_PUT_CHAR(sport, '\r');
- }
- }
+ uart_console_write(&sport->port, s, count, sa1100_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1729,6 +1729,27 @@ static int uart_read_proc(char *page, ch
#ifdef CONFIG_SERIAL_CORE_CONSOLE
/*
+ * uart_console_write - write a console message to a serial port
+ * @port: the port to write the message
+ * @s: array of characters
+ * @count: number of characters in string to write
+ * @write: function to write character to port
+ */
+void uart_console_write(struct uart_port *port, const char *s,
+ unsigned int count,
+ void (*write)(struct uart_port *, char))
+{
+ unsigned int i;
+
+ for (i = 0; i < count; i++, s++) {
+ if (*s == '\n')
+ write(port, '\r');
+ write(port, '\n');
+ }
+}
+EXPORT_SYMBOL(uart_console_write);
+
+/*
* Check whether an invalid uart number has been specified, and
* if so, search for the first available port that does have
* console support.
diff --git a/drivers/serial/serial_lh7a40x.c b/drivers/serial/serial_lh7a40x.c
--- a/drivers/serial/serial_lh7a40x.c
+++ b/drivers/serial/serial_lh7a40x.c
@@ -543,6 +543,12 @@ static struct uart_port_lh7a40x lh7a40x_
#else
# define LH7A40X_CONSOLE &lh7a40x_console
+static void lh7a40xuart_console_putchar(struct uart_port *port, char ch)
+{
+ while (UR(port, UART_R_STATUS) & nTxRdy)
+ ;
+ UR(port, UART_R_DATA) = ch;
+}
static void lh7a40xuart_console_write (struct console* co,
const char* s,
@@ -556,16 +562,7 @@ static void lh7a40xuart_console_write (s
UR (port, UART_R_INTEN) = 0; /* Disable all interrupts */
BIT_SET (port, UART_R_CON, UARTEN | SIRDIS); /* Enable UART */
- for (; count-- > 0; ++s) {
- while (UR (port, UART_R_STATUS) & nTxRdy)
- ;
- UR (port, UART_R_DATA) = *s;
- if (*s == '\n') {
- while ((UR (port, UART_R_STATUS) & TxBusy))
- ;
- UR (port, UART_R_DATA) = '\r';
- }
- }
+ uart_console_write(port, s, count, lh7a40xuart_console_putchar);
/* Wait until all characters are sent */
while (UR (port, UART_R_STATUS) & TxBusy)
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -854,6 +854,14 @@ static inline void wait_for_xmitr(struct
}
}
+static void serial_txx9_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_txx9_port *up = (struct uart_txx9_port *)port;
+
+ wait_for_xmitr(up);
+ sio_out(up, TXX9_SITFIFO, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -865,7 +873,6 @@ serial_txx9_console_write(struct console
{
struct uart_txx9_port *up = &serial_txx9_ports[co->index];
unsigned int ier, flcr;
- int i;
/*
* First save the UER then disable the interrupts
@@ -879,22 +886,7 @@ serial_txx9_console_write(struct console
if (!(up->port.flags & UPF_CONS_FLOW) && (flcr & TXX9_SIFLCR_TES))
sio_out(up, TXX9_SIFLCR, flcr & ~TXX9_SIFLCR_TES);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++, s++) {
- wait_for_xmitr(up);
-
- /*
- * Send the character out.
- * If a LF, also do CR...
- */
- sio_out(up, TXX9_SITFIFO, *s);
- if (*s == 10) {
- wait_for_xmitr(up);
- sio_out(up, TXX9_SITFIFO, 13);
- }
- }
+ uart_console_write(&up->port, s, count, serial_txx9_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/sunsab.c b/drivers/serial/sunsab.c
--- a/drivers/serial/sunsab.c
+++ b/drivers/serial/sunsab.c
@@ -861,8 +861,9 @@ static int num_channels;
#ifdef CONFIG_SERIAL_SUNSAB_CONSOLE
-static __inline__ void sunsab_console_putchar(struct uart_sunsab_port *up, char c)
+static void sunsab_console_putchar(struct uart_port *port, char c)
{
+ struct uart_sunsab_port *up = (struct uart_sunsab_port *)port;
unsigned long flags;
spin_lock_irqsave(&up->port.lock, flags);
@@ -876,13 +877,8 @@ static __inline__ void sunsab_console_pu
static void sunsab_console_write(struct console *con, const char *s, unsigned n)
{
struct uart_sunsab_port *up = &sunsab_ports[con->index];
- int i;
- for (i = 0; i < n; i++) {
- if (*s == '\n')
- sunsab_console_putchar(up, '\r');
- sunsab_console_putchar(up, *s++);
- }
+ uart_console_write(&up->port, s, n, sunsab_console_putchar);
sunsab_tec_wait(up);
}
diff --git a/drivers/serial/sunsu.c b/drivers/serial/sunsu.c
--- a/drivers/serial/sunsu.c
+++ b/drivers/serial/sunsu.c
@@ -1379,6 +1379,14 @@ static __inline__ void wait_for_xmitr(st
}
}
+static void sunsu_console_putchar(struct uart_port *port, char ch)
+{
+ struct uart_sunsu_port *up = (struct uart_sunsu_port *)port;
+
+ wait_for_xmitr(up);
+ serial_out(up, UART_TX, ch);
+}
+
/*
* Print a string to the serial port trying not to disturb
* any possible real use of the port...
@@ -1388,7 +1396,6 @@ static void sunsu_console_write(struct c
{
struct uart_sunsu_port *up = &sunsu_ports[co->index];
unsigned int ier;
- int i;
/*
* First save the UER then disable the interrupts
@@ -1396,22 +1403,7 @@ static void sunsu_console_write(struct c
ier = serial_in(up, UART_IER);
serial_out(up, UART_IER, 0);
- /*
- * Now, do each character
- */
- for (i = 0; i < count; i++, s++) {
- wait_for_xmitr(up);
-
- /*
- * Send the character out.
- * If a LF, also do CR...
- */
- serial_out(up, UART_TX, *s);
- if (*s == 10) {
- wait_for_xmitr(up);
- serial_out(up, UART_TX, 13);
- }
- }
+ uart_console_write(&up->port, s, count, sunsu_console_putchar);
/*
* Finally, wait for transmitter to become empty
diff --git a/drivers/serial/sunzilog.c b/drivers/serial/sunzilog.c
--- a/drivers/serial/sunzilog.c
+++ b/drivers/serial/sunzilog.c
@@ -1331,9 +1331,9 @@ sunzilog_console_write(struct console *c
spin_lock_irqsave(&up->port.lock, flags);
for (i = 0; i < count; i++, s++) {
+ if (*s == '\n')
+ sunzilog_put_char(channel, '\r');
sunzilog_put_char(channel, *s);
- if (*s == 10)
- sunzilog_put_char(channel, 13);
}
udelay(2);
spin_unlock_irqrestore(&up->port.lock, flags);
diff --git a/drivers/serial/vr41xx_siu.c b/drivers/serial/vr41xx_siu.c
--- a/drivers/serial/vr41xx_siu.c
+++ b/drivers/serial/vr41xx_siu.c
@@ -821,25 +821,23 @@ static void wait_for_xmitr(struct uart_p
}
}
+static void siu_console_putchar(struct uart_port *port, char ch)
+{
+ wait_for_xmitr(port);
+ siu_write(port, UART_TX, ch);
+}
+
static void siu_console_write(struct console *con, const char *s, unsigned count)
{
struct uart_port *port;
uint8_t ier;
- unsigned i;
port = &siu_uart_ports[con->index];
ier = siu_read(port, UART_IER);
siu_write(port, UART_IER, 0);
- for (i = 0; i < count && *s != '\0'; i++, s++) {
- wait_for_xmitr(port);
- siu_write(port, UART_TX, *s);
- if (*s == '\n') {
- wait_for_xmitr(port);
- siu_write(port, UART_TX, '\r');
- }
- }
+ uart_console_write(port, s, count, siu_console_putchar);
wait_for_xmitr(port);
siu_write(port, UART_IER, ier);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -366,6 +366,9 @@ void uart_parse_options(char *options, i
int uart_set_options(struct uart_port *port, struct console *co, int baud,
int parity, int bits, int flow);
struct tty_driver *uart_console_device(struct console *co, int *index);
+void uart_console_write(struct uart_port *port, const char *s,
+ unsigned int count,
+ void (*write)(struct uart_port *, char));
/*
* Port/driver registration/removal
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King <[email protected]> writes:
> Stop throwing FUD into this issue. The original claim was that a
> non-root user could send arbitary strings via the console system.
> This was an independent claim from the other issues.
That is clearly another thing and would probably need to be
demonstrated. Anyway, the AT* problem[1] doesn't require a user
action and the fix proposed (if correct) would fix the real AT*
problem instead.
[1] I don't call it a bug, it's rather a lack of functionality,
we just don't seem to support dial-in Hayes modems with serial
console.
I don't see any FUD here.
--
Krzysztof Halasa
Russell King <[email protected]> writes:
> What about those who have incomplete null modem cables which might
> not connect DCD or DSR, but who want to use hardware flow control?
BTW: Obviously CRTSCTS is a different thing than a modem with
hardware handshaking. Basically CRTSCTS is a fixed, transparent
line. So if we do Hayes modem console, it would better be another
option.
--
Krzysztof Halasa
On Sat, Feb 04, 2006 at 05:18:05PM +0100, Krzysztof Halasa wrote:
> Russell King <[email protected]> writes:
>
> > What about those who have incomplete null modem cables which might
> > not connect DCD or DSR, but who want to use hardware flow control?
>
> BTW: Obviously CRTSCTS is a different thing than a modem with
> hardware handshaking. Basically CRTSCTS is a fixed, transparent
> line. So if we do Hayes modem console, it would better be another
> option.
I'm sorry, I don't understand what you're saying.
CRTSCTS is to enable RTS/CTS hardware handshaking for the tty, which
is what the modem wants if it is doing hardware handshaking. Why
should we invent a non-standard option to enable RTS/CTS hardware
handshaking when CRTSCTS is already defined to do this?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 04, 2006 at 05:08:10PM +0100, Krzysztof Halasa wrote:
> Russell King <[email protected]> writes:
>
> > Stop throwing FUD into this issue. The original claim was that a
> > non-root user could send arbitary strings via the console system.
> > This was an independent claim from the other issues.
>
> That is clearly another thing and would probably need to be
> demonstrated. Anyway, the AT* problem[1] doesn't require a user
> action and the fix proposed (if correct) would fix the real AT*
> problem instead.
The problem being discussed in this sub-thread was explicitly related
to just one case - where a _non root_ user may inject AT command
sequences. Your response in that thread was throwing up random
other issues, and in that respect it's just adding confusion to
the specific issue being discussed.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King <[email protected]> writes:
> CRTSCTS is to enable RTS/CTS hardware handshaking for the tty, which
> is what the modem wants if it is doing hardware handshaking.
Yes. But a Hayes modem wants a bit more than just RTS/CTS handshaking.
> Why
> should we invent a non-standard option to enable RTS/CTS hardware
> handshaking when CRTSCTS is already defined to do this?
That's not my idea.
If we want to support modems (Hayes-compatible) we need to make sure,
in addition to CRTSCTS, that we don't send anything when DCD (or DSR)
is down - and then we probably need another option.
BTW I think you know all of this very well for years.
--
Krzysztof Halasa
On Sun, Feb 05, 2006 at 12:54:51AM +0100, Krzysztof Halasa wrote:
> BTW I think you know all of this very well for years.
Sorry, with comments like that I'm not interested anymore.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King wrote:
> The problem being discussed in this sub-thread was explicitly related
> to just one case - where a _non root_ user may inject AT command
> sequences. Your response in that thread was throwing up random
> other issues, and in that respect it's just adding confusion to
> the specific issue being discussed.
Hello Krzysztof,
We closed that out. I was wrong. Covert channels are handled
by kernel programmers being careful not to put user-controllable
data in kprintf() stings. And that makes a lot of sense if you
think about the havoc that covert channels could also cause with
ANSI console (and their redefinable keys) and UTF-8 consoles
(and that character set's large number of easily-confused glyphs).
It might be nice to have the kernel audited for such kprintf()
strings but that is not something that needs discussion with
Russell.
I'm happy to admit error -- I don't do kernel coding and I expect
to make some mistakes. I just hope to handle them graciously.
With that in mind I'd like to apologise to Russell for claiming
it was a bug.
The serial console still should not write into an unasserted
DSR or DCD since that will hang up on incoming calls. But
they's a totally different, non-security issue. Which isn't
to say that it's not frustrating for the sysadmin who can't
connect to their misbehaving system.
I think the open issues are:
1. kernel messages causing modems to hang up during
the initial training period of the connection.
2. slow kernel boot times with 'r' option.
For (2) Russell is saying "don't do that". I am struggling
with what is the meaning of the 'r' option then. If it is
"I want my kernel console to try to not drop any kernel
messages" then is it reasonable to restrict that to directly
connected machines and disallow the use of the facility with
modems and console servers?
I'm also unsure of the robustness equipment in the face
of Russell's suggestion. The suggestion implies that the
kernel will write strings when CTS is unasserted. There
is some allowance for that in receivers that are configured
for RTS/CTS flow control, but it that allowance being overly
stressed? And does it matter if the modem or the receiver
drops some characters? Is there popular equipment for which
this is a pathological case?
I don't know the answer and have not had much time to think
Russell's suggestion through properly and no time at all
to run it against some popular modems and terminal servers.
Which is why I have not replied. I am spending the next two
days on airplanes, so there is plenty of time for thinking
then but no time for testing.
I would probably prefer two flags -- 'r' meaning flow control
(CTS) and a new option, say 's', meaning modem status (DSR,DCD).
That would nicely allow backwardly-compatible behaviour and
support a wide range of RS-232 cables and equipment. But
I want think through and test Russell's suggested interpretation
of the 'r' flag first.
> BTW I think you know all of this very well for years.
I don't know what you were thinking when you wrote this. But
it is stupid. It's one thing to be technically wrong -- all
that is required to fix that is some patience on both sides.
And Russell has been very patient with me, which I appreciate.
It's totally another thing entirely to be insulting. What
do you now think are the chances of people working together
harmoniously to have the open issues satisfactorily resolved?
What do I say to the readers of the HOWTO about the shortcomings
of the serial console implementation? I can hardly write that
LKML discussed the issue, but the participants in the mailing
list preferred sly insults over closing the issues that HOWTO
readers had reported.
I don't in any way have a problem with the years this has
taken to get an airing. I have a day job, and that is not
kernel patch wrangling, so it is not like people are going
to even notice my e-mail in the noise of LKML. I am not
known to the community. My code probably sucks, despite my
best efforts. I expect people to be questioning, just as
I am of e-mails from unknown people with 'improvements' to
router behaviour.
Regards,
Glen
--
Glen Turner Tel: (08) 8303 3936 or +61 8 8303 3936
Australia's Academic & Research Network http://www.aarnet.edu.au
Russell King <[email protected]> writes:
>> BTW I think you know all of this very well for years.
>
> Sorry, with comments like that I'm not interested anymore.
No offense intended, I just thought you were using this stuff (in the
BBSes era etc.). I must have been wrong then.
--
Krzysztof Halasa
Hi,
Glen Turner <[email protected]> writes:
> The serial console still should not write into an unasserted
> DSR or DCD since that will hang up on incoming calls. But
> they's a totally different, non-security issue.
Could be a security issue if the modem gets reconfigured.
> I'm also unsure of the robustness equipment in the face
> of Russell's suggestion. The suggestion implies that the
> kernel will write strings when CTS is unasserted. There
> is some allowance for that in receivers that are configured
> for RTS/CTS flow control, but it that allowance being overly
> stressed? And does it matter if the modem or the receiver
> drops some characters? Is there popular equipment for which
> this is a pathological case?
Actually I think you can't currently connect a Hayes modem directly
to a serial console, CRTSCTS or no flow control.
With terminal server it may work (especially Linux-based or similar),
for example, with "screen".
> I would probably prefer two flags -- 'r' meaning flow control
> (CTS) and a new option, say 's', meaning modem status (DSR,DCD).
Would make sense I think.
>> BTW I think you know all of this very well for years.
>
> I don't know what you were thinking when you wrote this. But
> it is stupid. It's one thing to be technically wrong -- all
> that is required to fix that is some patience on both sides.
> And Russell has been very patient with me, which I appreciate.
>
> It's totally another thing entirely to be insulting. What
> do you now think are the chances of people working together
> harmoniously to have the open issues satisfactorily resolved?
I'm not exactly sure what do you mean. First I thought he was
overworked or something like that but now I wonder if I should
check it with an English teacher nearby?
I just really thought he knows modem control lines details and
how Hayes modems behave etc. from BBS era.
What exactly is the "insult" here?
--
Krzysztof Halasa
On Sun, Feb 05, 2006 at 01:42:20PM +1030, Glen Turner wrote:
> I think the open issues are:
>
> 1. kernel messages causing modems to hang up during
> the initial training period of the connection.
I think the correct solution is as suggested - a new option to enable
DCD monitoring, which when specified and DCD is not asserted we avoid
transmitting any kernel messages. The side effect of this is that
any kernel messages avoided will be lost, and that point needs to be
made clear. However, they will still be accessible via dmesg /
syslog.
> 2. slow kernel boot times with 'r' option.
The 'r' option is there to provide a reasonably reliable form of flow
control for kernel messages, so that kernel messages can reach the
receiver intact. Strictly, if 'r' is specified and CTS is deasserted,
we should not transmit _anything_ until CTS is asserted, but that
would mean effectively pausing the kernel indefinitely, so there's
a limit on how long we wait for CTS.
I think we're agreed (differently wired serial cables) as to why using
the 'r' option to also monitor DSR would be a bad idea.
A possible solution to this problem may be to have the additional
option as you have suggested. Whether it should be the same option
as (1) above or not is debatable.
The advantage of the CRLF patch I posted previously is that we can now
do a lot of the above in common code, which will fix a lot of serial
drivers at the same time.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Hi!
> > The serial console driver has a host of issues
> >
> > [...]
> >
> > - [SECURITY] 'r' should require DCD to be asserted
> > before outputing characters. Otherwise we talk to
> > Hayes modem command mode. This allows a non-root
> > user to re-program the modem and is a major security
> > issue is people configure calling line identification
> > or encryption to restrict use of the serial console.
>
> How is this possible? A normal user can't produce arbitarily formatted
> kernel messages, and if they have access to /dev/ttyS they can do what
> ever they like with the port anyway.
Maybe not *arbitrary* messages, but any user probably can fake enough
to
confuse modem. Name your process \nATD609123456\n and cause it to eat
all memory, or something like that. OOM killer will print name...
Pavel
--
Thanks, Sharp!
On Mon, Feb 06, 2006 at 08:26:55PM +0000, Pavel Machek wrote:
> Hi!
>
> > > The serial console driver has a host of issues
> > >
> > > [...]
> > >
> > > - [SECURITY] 'r' should require DCD to be asserted
> > > before outputing characters. Otherwise we talk to
> > > Hayes modem command mode. This allows a non-root
> > > user to re-program the modem and is a major security
> > > issue is people configure calling line identification
> > > or encryption to restrict use of the serial console.
> >
> > How is this possible? A normal user can't produce arbitarily formatted
> > kernel messages, and if they have access to /dev/ttyS they can do what
> > ever they like with the port anyway.
>
> Maybe not *arbitrary* messages, but any user probably can fake enough
> to
> confuse modem. Name your process \nATD609123456\n and cause it to eat
> all memory, or something like that. OOM killer will print name...
As I say, it's a problem which needs fixing elsewhere. What if
the process was called:
\nSystem Halted\n
(which will fit in the kernel's process name.) Or maybe some escape
sequence which reprograms your terminal.
As I say, this is a problem which needs solving by other means. Maybe
flush_old_exec() should be a little more careful about what it copies,
changing non-alphanumeric characters to '?' ?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Hi Russell,
> I think we're agreed (differently wired serial cables) as to why using
> the 'r' option to also monitor DSR would be a bad idea.
We are. We need a caveat in the documentation reminding
users of 'r' the consequences of letting CTS float, but
that is all.
> A possible solution to this problem may be to have the additional
> option as you have suggested. Whether it should be the same option
> as (1) above or not is debatable.
I think the same option will work -- let's define 'm' as
being "strictly observe modem status signals (ie, DSR and
DCD)".
'm' them implies:
- all other modem signals (CTS and DCD) are undefined
if DSR is not asserted.
- transmission will only be allowed if DSR and DCD are
both asserted at the instant when the character is
ready for transmission. There may be additional
restrictions on transmission from flow control.
Unlike flow control, the kernel never waits for
the modem status signals to become asserted, but
instantly discards the character.
'r' remains the same, namely
- if CTS is asserted then the character is transmitted.
Noting that 'm' additionally implies that CTS is only
defined when DSR is asserted.
- if 'r' is specificed and CTS is not asserted the
transmitter will wait a limited period for CTS to
become asserted. If CTS is not asserted within this time
then the character is discarded.
[As you can see I've changed the suggested option character. Perhaps
's' as an option is just too easily confused with the 's' in 'ttyS'.]
> The advantage of the CRLF patch I posted previously is that we can now
> do a lot of the above in common code, which will fix a lot of serial
> drivers at the same time.
I had noticed that and it's a fine idea.
Depending on your timetable, if you agree with the above and
wait a few days I'll send a patch building atop your patch
and implementing the above. I'm sorry I can't do that at
this moment, but I'm bandwidth-deprived. Of course, being
much more experienced in this sort of thing you may have
already whipped something up :-)
Regards,
Glen
--
Glen Turner Tel: (08) 8303 3936 or +61 8 8303 3936
Australia's Academic & Research Network http://www.aarnet.edu.au
[Noting that I know next-to-nothing about kernel programming,
but I have been down this particular road before...]
Russell King wrote:
> Maybe flush_old_exec() should be a little more careful
> about what it copies, changing non-alphanumeric characters
> to '?' ?
I'm not sure it can do that, if the kernel policy is to
be 8-bit clean (to allow UTF-8 to work without coding
UTF-8 knowledge into the kernel).
What the code could do is not printk() user-influenced strings
at all. For example, mm/oom_kill.c could print just the process
ID here:
printk(KERN_ERR "Out of Memory: Killed process %d (%s).\n",
p->pid, p->comm);
The usual solution to this problem is to mark user-derived
strings as tainted and then check for the taint attribute
when strings are requested to be output. But since this
is a kernel I don't suppose you'd be keen doing that :-)
I suppose you need a policy decision -- are strings scrubbed
on input (I've coded this once and it is really quite tricky).
And then do you need a scrubbed and non-scrubbed version of
p->comm (as comparing scrubbed p->comm for equality is
problematic and probably expolitable). Or do you simply not
output strings which have been tainted by contact with users.
--
Glen Turner Tel: (08) 8303 3936 or +61 8 8303 3936
Australia's Academic & Research Network http://www.aarnet.edu.au
On Po 06-02-06 20:55:00, Russell King wrote:
> On Mon, Feb 06, 2006 at 08:26:55PM +0000, Pavel Machek wrote:
> > Hi!
> >
> > > > The serial console driver has a host of issues
> > > >
> > > > [...]
> > > >
> > > > - [SECURITY] 'r' should require DCD to be asserted
> > > > before outputing characters. Otherwise we talk to
> > > > Hayes modem command mode. This allows a non-root
> > > > user to re-program the modem and is a major security
> > > > issue is people configure calling line identification
> > > > or encryption to restrict use of the serial console.
> > >
> > > How is this possible? A normal user can't produce arbitarily formatted
> > > kernel messages, and if they have access to /dev/ttyS they can do what
> > > ever they like with the port anyway.
> >
> > Maybe not *arbitrary* messages, but any user probably can fake enough
> > to
> > confuse modem. Name your process \nATD609123456\n and cause it to eat
> > all memory, or something like that. OOM killer will print name...
>
> As I say, it's a problem which needs fixing elsewhere. What if
> the process was called:
>
> \nSystem Halted\n
>
> (which will fit in the kernel's process name.) Or maybe some escape
> sequence which reprograms your terminal.
>
> As I say, this is a problem which needs solving by other means. Maybe
> flush_old_exec() should be a little more careful about what it copies,
> changing non-alphanumeric characters to '?' ?
I'm afraid that auditing kernel to never ever print \n from user will
be quite a long job. If I get
Killed process 1234
System Halted
due to OOM
I am going to figure it out no problem, but modems do not have that
kind of abilities...
Pavel
--
Web maintainer for suspend.sf.net (http://www.sf.net/projects/suspend) wanted...
On Tue, Feb 07, 2006 at 10:18:04AM +0100, Pavel Machek wrote:
> I'm afraid that auditing kernel to never ever print \n from user will
> be quite a long job. If I get
>
> Killed process 1234
> System Halted
> due to OOM
>
> I am going to figure it out no problem, but modems do not have that
> kind of abilities...
In that case the problem is unsolvable. What if I named a process
\n+++ATH0\n
? Oh dear, your modem just hung up. Or maybe:
\n+++AT&C0\n
and now your modem always sets DCD active, so even with detection of DCD
in the kernel, I can now talk to it via process names after I've forced
it to disconnect.
And yes, there's modems out there which accept that and act on the '+++'
immediately - no pause after '+++' required.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King <[email protected]> writes:
> In that case the problem is unsolvable. What if I named a process
>
> \n+++ATH0\n
>
> ? Oh dear, your modem just hung up. Or maybe:
>
> \n+++AT&C0\n
>
> and now your modem always sets DCD active, so even with detection of DCD
> in the kernel, I can now talk to it via process names after I've forced
> it to disconnect.
>
> And yes, there's modems out there which accept that and act on the '+++'
> immediately - no pause after '+++' required.
Correct, but the escape character can usually be disabled with ATS2=128
or something like that.
The problem was common some time ago with people putting X-header: +++ATH\n
(or was that +++ATH\r ?) in mail headers.
--
Krzysztof Halasa
Russell King <[email protected]> writes:
> And yes, there's modems out there which accept that and act on the '+++'
> immediately - no pause after '+++' required.
Still? Heatherington patent (U.S. patent #4549302) was granted
in October 1985, hasn't it already expired?
On Tue, 2006-02-07 at 16:59 -0800, Junio C Hamano wrote:
> Russell King <[email protected]> writes:
>
> > And yes, there's modems out there which accept that and act on the '+++'
> > immediately - no pause after '+++' required.
>
> Still? Heatherington patent (U.S. patent #4549302) was granted
> in October 1985, hasn't it already expired?
Yes but everything on the market was likely designed before it expired.
Lee