[PATCH] serial_core: avoid Break bouncing

[PATCH] serial_core: avoid Break bouncing

[Mauro Carvalho Chehab]

From: Eran Liberty <liberty@extricom.com>

On some boxes, Break signal bounces, causing sysrq code to fail with some
serial interfaces.

A solution were posted on LKML in 2008:
http://lkml.indiana.edu/hypermail/linux/kernel/0809.2/0730.html

However, the fix weren't applied upstream.

This bug keeps happening, as shown at:
	https://bugzilla.redhat.com/show_bug.cgi?id=518120

The original patch from Eran adds a debouncing logic that avoids that
multiple breaks to be badly handled by the serial code.

[mchehab@redhat.com: port the patch upstream and fix CodingStyle]

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index db532ce..765e169 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -443,8 +443,8 @@ static inline int
 uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
 {
 #ifdef SUPPORT_SYSRQ
-	if (port->sysrq) {
-		if (ch && time_before(jiffies, port->sysrq)) {
+	if (port->sysrq && time_after(jiffies, port->sysrq + HZ / 50)) {
+		if (ch && time_before(jiffies, port->sysrq + HZ * 5)) {
 			handle_sysrq(ch, port->state->port.tty);
 			port->sysrq = 0;
 			return 1;
@@ -464,18 +464,17 @@ uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
 static inline int uart_handle_break(struct uart_port *port)
 {
 	struct uart_state *state = port->state;
+	int ret = 0;
+
 #ifdef SUPPORT_SYSRQ
 	if (port->cons && port->cons->index == port->line) {
-		if (!port->sysrq) {
-			port->sysrq = jiffies + HZ*5;
-			return 1;
-		}
-		port->sysrq = 0;
+		port->sysrq = jiffies;
+		ret = 1;
 	}
 #endif
 	if (port->flags & UPF_SAK)
 		do_SAK(state->port.tty);
-	return 0;
+	return ret;
 }
 
 /**

Re: [PATCH] serial_core: avoid Break bouncing

[Alan Cox]

On Thu, 12 Nov 2009 14:05:04 -0200
Mauro Carvalho Chehab <mchehab@redhat.com> wrote:

> From: Eran Liberty <liberty@extricom.com>
> 
> On some boxes, Break signal bounces, causing sysrq code to fail with some
> serial interfaces.
> 
> A solution were posted on LKML in 2008:
> http://lkml.indiana.edu/hypermail/linux/kernel/0809.2/0730.html
> 
> However, the fix weren't applied upstream.

Because it was NAKked at the time as it seemed to be a bug in Eran's
hardware platform not a generic problem and also because it breaks
autobauding on break as used by some (particularly older) software.

It's still NAKked and it would be helpful if people resubmitting old
stuff also looked through the thread attached the original submission.

You seem to have a single case of a user with some kind of buggy uart or
possibly other weirdness going on. That's probably worth investigating
more deeply but the patch NAKked before is not the solution due to its
side effects

(also btw it'll upset other people who script their sysrq stuff including
breaks and don't expect strange timing limits)

NAK again

Re: [PATCH] serial_core: avoid Break bouncing

[Mauro Carvalho Chehab]

Em Thu, 12 Nov 2009 17:56:49 +0000
Alan Cox <alan@lxorguk.ukuu.org.uk> escreveu:

> On Thu, 12 Nov 2009 14:05:04 -0200
> Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
> 
> > From: Eran Liberty <liberty@extricom.com>
> > 
> > On some boxes, Break signal bounces, causing sysrq code to fail with some
> > serial interfaces.
> > 
> > A solution were posted on LKML in 2008:
> > http://lkml.indiana.edu/hypermail/linux/kernel/0809.2/0730.html
> > 
> > However, the fix weren't applied upstream.
> 
> Because it was NAKked at the time as it seemed to be a bug in Eran's
> hardware platform not a generic problem and also because it breaks
> autobauding on break as used by some (particularly older) software.
> 
> It's still NAKked and it would be helpful if people resubmitting old
> stuff also looked through the thread attached the original submission.

I did, but I haven't find any nack at the public archives I've checked. My
fault. Sorry about that.

> You seem to have a single case of a user with some kind of buggy uart or
> possibly other weirdness going on. That's probably worth investigating
> more deeply but the patch NAKked before is not the solution due to its
> side effects

I'll do a deeper analysis and come back with a patch only if I discover
something new about this issue.
> 
> (also btw it'll upset other people who script their sysrq stuff including
> breaks and don't expect strange timing limits)
> 
> NAK again


-- 

Cheers,
Mauro

Re: [PATCH] serial_core: avoid Break bouncing

[Alan Cox]

> I'll do a deeper analysis and come back with a patch only if I discover
> something new about this issue.

I'll forward you the end of the previous discussion

Re: [PATCH] serial_core: avoid Break bouncing

[Eran Liberty]

Alan Cox wrote:
>> I'll do a deeper analysis and come back with a patch only if I discover
>> something new about this issue.
>>     
>
> I'll forward you the end of the previous discussion
>   

Tried, unsuccessfully , to dig up the end of the original thread. In the 
end my patch was deemed unworthy for:
1. Creating a security whole. I am not a security expert so I just 
accepted it as is.
2. Trying to fix something that is not broken. i.e. multiple entry to 
uart_handle_break() upon break trigger is a localized problem to my 
hardware.

If something changed and you need my help I will be more then happy to 
extend it.

-- Liberty