* [PATCH 0/2] more usb sysrq improvements
@ 2009-05-29 18:34 Jason Wessel
2009-05-29 18:34 ` [PATCH 1/2] pl2303 usb_serial: implement sysrq handling on break Jason Wessel
0 siblings, 1 reply; 9+ messages in thread
From: Jason Wessel @ 2009-05-29 18:34 UTC (permalink / raw)
To: greg; +Cc: linux-kernel, linux-usb
This patch series is to add sysrq support for the pl2303 usb device
and to stop sysrq processing on non-console usb serial ports.
Thanks,
Jason.
The short log and git info is below, which is against the 2.6.30
development tree with the previous 4 accepted usb patches in the
gregkh queue.
The following changes since commit a425a638c858fd10370b573bde81df3ba500e271:
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_gregkh_2
Jason Wessel (2):
pl2303 usb_serial: implement sysrq handling on break
usb_serial: only allow sysrq on a console port
drivers/usb/serial/generic.c | 2 +-
drivers/usb/serial/pl2303.c | 5 ++++-
2 files changed, 5 insertions(+), 2 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] pl2303 usb_serial: implement sysrq handling on break
2009-05-29 18:34 [PATCH 0/2] more usb sysrq improvements Jason Wessel
@ 2009-05-29 18:34 ` Jason Wessel
2009-05-29 18:34 ` [PATCH 2/2] usb_serial: only allow sysrq on a console port Jason Wessel
0 siblings, 1 reply; 9+ messages in thread
From: Jason Wessel @ 2009-05-29 18:34 UTC (permalink / raw)
To: greg; +Cc: linux-kernel, linux-usb, Jason Wessel
Add callbacks to process the sysrq when using a pl2303 usb device as a
console.
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
drivers/usb/serial/pl2303.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 751a533..c5a8745 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -946,6 +946,8 @@ static void pl2303_update_line_status(struct usb_serial_port *port,
spin_lock_irqsave(&priv->lock, flags);
priv->line_status = data[status_idx];
spin_unlock_irqrestore(&priv->lock, flags);
+ if (priv->line_status & UART_BREAK_ERROR)
+ usb_serial_handle_break(port);
wake_up_interruptible(&priv->delta_msr_wait);
}
@@ -1056,7 +1058,8 @@ static void pl2303_read_bulk_callback(struct urb *urb)
if (line_status & UART_OVERRUN_ERROR)
tty_insert_flip_char(tty, 0, TTY_OVERRUN);
for (i = 0; i < urb->actual_length; ++i)
- tty_insert_flip_char(tty, data[i], tty_flag);
+ if (!usb_serial_handle_sysrq_char(port, data[i]))
+ tty_insert_flip_char(tty, data[i], tty_flag);
tty_flip_buffer_push(tty);
}
tty_kref_put(tty);
--
1.6.3.1.9.g95405b
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-05-29 18:34 ` [PATCH 1/2] pl2303 usb_serial: implement sysrq handling on break Jason Wessel
@ 2009-05-29 18:34 ` Jason Wessel
2009-06-01 13:35 ` Alan Cox
0 siblings, 1 reply; 9+ messages in thread
From: Jason Wessel @ 2009-05-29 18:34 UTC (permalink / raw)
To: greg; +Cc: linux-kernel, linux-usb, Jason Wessel
The only time a sysrq should get processed is if the attached device
is a console. This is intended to protect sysrq execution on a host
connected with a terminal program.
Here is the problem scenario:
host A <-- rs232 link --> host B
Host A is using mincom and a usb pl2303 device to connect to host b
which is a linux system with a usb pl2303 device acting as the serial
console. When host B is rebooted the pl2303 emits random junk
characters on reset. These character sequences contain serial break
signals most of the time and when translated to a sysrq have caused
host A to get random processes killed, reboots or power down.
It is true that in this setup with this patch host B might still have
the same problem as host A if you reboot host A. In most cases host A
is a development host which seldom gets rebooted, and you could turn
off sysrq temporarily on host B if you need to reboot host A.
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
drivers/usb/serial/generic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index df56ed3..4797402 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -529,7 +529,7 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
int usb_serial_handle_sysrq_char(struct usb_serial_port *port, unsigned int ch)
{
- if (port->sysrq) {
+ if (port->sysrq && port->console) {
if (ch && time_before(jiffies, port->sysrq)) {
handle_sysrq(ch, tty_port_tty_get(&port->port));
port->sysrq = 0;
--
1.6.3.1.9.g95405b
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-05-29 18:34 ` [PATCH 2/2] usb_serial: only allow sysrq on a console port Jason Wessel
@ 2009-06-01 13:35 ` Alan Cox
2009-07-05 16:54 ` Robin Getz
0 siblings, 1 reply; 9+ messages in thread
From: Alan Cox @ 2009-06-01 13:35 UTC (permalink / raw)
To: Jason Wessel; +Cc: greg, linux-kernel, linux-usb, Jason Wessel
On Fri, 29 May 2009 13:34:17 -0500
Jason Wessel <jason.wessel@windriver.com> wrote:
> The only time a sysrq should get processed is if the attached device
> is a console. This is intended to protect sysrq execution on a host
> connected with a terminal program.
This doesn't seem to match any tree I can find ?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-06-01 13:35 ` Alan Cox
@ 2009-07-05 16:54 ` Robin Getz
2009-07-05 18:17 ` Alan Cox
0 siblings, 1 reply; 9+ messages in thread
From: Robin Getz @ 2009-07-05 16:54 UTC (permalink / raw)
To: Alan Cox, greg; +Cc: Jason Wessel, linux-kernel, linux-usb
On Mon 1 Jun 2009 09:35, Alan Cox pondered:
> On Fri, 29 May 2009 13:34:17 -0500
> Jason Wessel <jason.wessel@windriver.com> wrote:
>
> > The only time a sysrq should get processed is if the attached device
> > is a console. This is intended to protect sysrq execution on a host
> > connected with a terminal program.
>
> This doesn't seem to match any tree I can find ?
Alan:
If Jason's patch is necessary () - should this be fixed up for standard
UARTs too?
Make sure that only serial console (not _any_ serial port) responds to
sysrq (or should something else be ensuring that this isn't set when
the port !console? (I didn't see anything in serial_core.c?)
---
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 23d2fb0..f8ab858 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -474,7 +474,7 @@ static inline int
uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
{
#ifdef SUPPORT_SYSRQ
- if (port->sysrq) {
+ if (port->sysrq && port->console) {
if (ch && time_before(jiffies, port->sysrq)) {
handle_sysrq(ch, port->info->port.tty);
port->sysrq = 0;
----------
That brings up the next question...
The above patch would sync the (seemlying duplicated) code between
drivers/usb/serial/generic.c and include/linux/serial_core.h
Greg?
drivers/usb/serial/generic.c
int usb_serial_handle_sysrq_char(struct usb_serial_port *port, unsigned int ch)
{
if (port->sysrq && port->console) {
if (ch && time_before(jiffies, port->sysrq)) {
handle_sysrq(ch, tty_port_tty_get(&port->port));
port->sysrq = 0;
return 1;
}
port->sysrq = 0;
}
return 0;
}
include/linux/serial_core.h
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)) {
handle_sysrq(ch, port->info->port.tty);
port->sysrq = 0;
return 1;
}
port->sysrq = 0;
}
#endif
return 0;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-07-05 16:54 ` Robin Getz
@ 2009-07-05 18:17 ` Alan Cox
2009-07-06 4:38 ` Robin Getz
0 siblings, 1 reply; 9+ messages in thread
From: Alan Cox @ 2009-07-05 18:17 UTC (permalink / raw)
To: Robin Getz; +Cc: greg, Jason Wessel, linux-kernel, linux-usb
> If Jason's patch is necessary () - should this be fixed up for standard
> UARTs too?
I think so yes, although I'd not realised it wasn't protected currently ?
>
> Make sure that only serial console (not _any_ serial port) responds to
> sysrq (or should something else be ensuring that this isn't set when
> the port !console? (I didn't see anything in serial_core.c?)
will check
> The above patch would sync the (seemlying duplicated) code between
> drivers/usb/serial/generic.c and include/linux/serial_core.h
There is a lot of near duplicate code like this. That is one reason for
adding struct tty_port. In theory both could be collapsed into
int tty_port_handle_sysrq(struct tty_port *port, unsigned int ch)
{
}
at this point as both USB and serial layer UARTs now have a port object.
That would just need port->sysrq collapsing into the tty_port.
port->console sort of already is.
> Greg?
>
> drivers/usb/serial/generic.c
> int usb_serial_handle_sysrq_char(struct usb_serial_port *port, unsigned int ch)
> {
> if (port->sysrq && port->console) {
> if (ch && time_before(jiffies, port->sysrq)) {
> handle_sysrq(ch, tty_port_tty_get(&port->port));
> port->sysrq = 0;
> return 1;
That also looks wrong - tty_port_tty_get takes a tty kref. I will check
that Monday and look at collapsing these int one as you note.
Alan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-07-05 18:17 ` Alan Cox
@ 2009-07-06 4:38 ` Robin Getz
2009-07-06 4:56 ` Mike Frysinger
0 siblings, 1 reply; 9+ messages in thread
From: Robin Getz @ 2009-07-06 4:38 UTC (permalink / raw)
To: Alan Cox; +Cc: greg, Jason Wessel, linux-kernel, linux-usb
On Sun 5 Jul 2009 14:17, Alan Cox pondered:
> > If Jason's patch is necessary () - should this be fixed up for
> > standard UARTs too?
>
> I think so yes, although I'd not realised it wasn't protected currently
> ?
Hmm - try as I may - I can't get this to fail - so it must be protected
somewhere....
Ahh---
It is in include/linux/serial_core.h:uart_handle_break() - never checked the
header for the magic before I bugged you... sorry about that...
> > The above patch would sync the (seemlying duplicated) code between
> > drivers/usb/serial/generic.c and include/linux/serial_core.h
>
> There is a lot of near duplicate code like this. That is one reason for
> adding struct tty_port. In theory both could be collapsed into
>
> int tty_port_handle_sysrq(struct tty_port *port, unsigned int ch)
> {
> }
>
> at this point as both USB and serial layer UARTs now have a port object.
> That would just need port->sysrq collapsing into the tty_port.
> port->console sort of already is.
It appears that the usb serial doesn't handle breaks like serial_core does. (I
don't see any support for SAK in usb_serial either?)
Maybe _that_ is the real problem that Jason is trying to work around???
> > drivers/usb/serial/generic.c
> > int usb_serial_handle_sysrq_char(struct usb_serial_port *port,
> unsigned int ch)
> > {
> > if (port->sysrq && port->console) {
> > if (ch && time_before(jiffies, port->sysrq)) {
> > handle_sysrq(ch,
> tty_port_tty_get(&port->port));
> > port->sysrq = 0;
> > return 1;
>
> That also looks wrong - tty_port_tty_get takes a tty kref. I will check
> that Monday and look at collapsing these int one as you note.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-07-06 4:38 ` Robin Getz
@ 2009-07-06 4:56 ` Mike Frysinger
2009-07-06 5:33 ` Robin Getz
0 siblings, 1 reply; 9+ messages in thread
From: Mike Frysinger @ 2009-07-06 4:56 UTC (permalink / raw)
To: Robin Getz; +Cc: Alan Cox, greg, Jason Wessel, linux-kernel, linux-usb
On Mon, Jul 6, 2009 at 00:38, Robin Getz wrote:
> On Sun 5 Jul 2009 14:17, Alan Cox pondered:
>> > If Jason's patch is necessary () - should this be fixed up for
>> > standard UARTs too?
>>
>> I think so yes, although I'd not realised it wasn't protected currently
>> ?
>
> Hmm - try as I may - I can't get this to fail - so it must be protected
> somewhere....
>
> Ahh---
>
> It is in include/linux/serial_core.h:uart_handle_break() - never checked the
> header for the magic before I bugged you... sorry about that...
>
>> > The above patch would sync the (seemlying duplicated) code between
>> > drivers/usb/serial/generic.c and include/linux/serial_core.h
>>
>> There is a lot of near duplicate code like this. That is one reason for
>> adding struct tty_port. In theory both could be collapsed into
>>
>> int tty_port_handle_sysrq(struct tty_port *port, unsigned int ch)
>> {
>> }
>>
>> at this point as both USB and serial layer UARTs now have a port object.
>> That would just need port->sysrq collapsing into the tty_port.
>> port->console sort of already is.
>
> It appears that the usb serial doesn't handle breaks like serial_core does. (I
> don't see any support for SAK in usb_serial either?)
>
> Maybe _that_ is the real problem that Jason is trying to work around???
perhaps, but what Jason proposed originally sounds pretty sane. if i
enable sysrq support on my desktop, i dont want some development board
being able to send a sysrq request back over my serial port and
rebooting my desktop.
-mike
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] usb_serial: only allow sysrq on a console port
2009-07-06 4:56 ` Mike Frysinger
@ 2009-07-06 5:33 ` Robin Getz
0 siblings, 0 replies; 9+ messages in thread
From: Robin Getz @ 2009-07-06 5:33 UTC (permalink / raw)
To: Mike Frysinger; +Cc: Alan Cox, greg, Jason Wessel, linux-kernel, linux-usb
On Mon 6 Jul 2009 00:56, Mike Frysinger pondered:
> On Mon, Jul 6, 2009 at 00:38, Robin Getz wrote:
> > It appears that the usb serial doesn't handle breaks like serial_core
> > does. (I don't see any support for SAK in usb_serial either?)
> >
> > Maybe _that_ is the real problem that Jason is trying to work around???
>
> perhaps, but what Jason proposed originally sounds pretty sane.
Which is why Greg added it to the USB tree, and it is in 2.6.31-rc2 :)
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=568d422e9cf52b7b26d2e026ae1617971f62b560
> if i
> enable sysrq support on my desktop, i dont want some development board
> being able to send a sysrq request back over my serial port and
> rebooting my desktop.
Fixing something in the wrong place -- while noble -- is still wrong :)
However -- with the way that usb serial is structured - I didn't see a better
place right now either.
The point being that is would be nice to make usb serial look more like other
serial devices - so they can share mode code - and things like this (that
have been in serial_core.h since -pre-git history (2005) wouldn't just be
getting added to the usb serial infrastructure now)...
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-07-06 5:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-29 18:34 [PATCH 0/2] more usb sysrq improvements Jason Wessel
2009-05-29 18:34 ` [PATCH 1/2] pl2303 usb_serial: implement sysrq handling on break Jason Wessel
2009-05-29 18:34 ` [PATCH 2/2] usb_serial: only allow sysrq on a console port Jason Wessel
2009-06-01 13:35 ` Alan Cox
2009-07-05 16:54 ` Robin Getz
2009-07-05 18:17 ` Alan Cox
2009-07-06 4:38 ` Robin Getz
2009-07-06 4:56 ` Mike Frysinger
2009-07-06 5:33 ` Robin Getz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.