Hey,
If no disconnection script provided and pppd being halted with SIGTERM,
CLOCAL is not re-set during disconnect_tty(). This leads to some nasty
race conditions during disconnection of mobile broadband modems in
NetworkManager/ModemManager.
Possible fix attached; which seems to solve the race conditions I'm
seeing in a slow ARM machine.
Some more background....
pppd will by default unset CLOCAL in the tty termios when connected, in
order to use modem control lines. But when pppd is stopped with SIGTERM,
CLOCAL is not being reset in the tty.
NetworkManager and ModemManager use pppd to setup the mobile broadband
modem connections. Once MM sets the tty in connected mode, NM launches
pppd in the tty and the session is established. When the user requests
to stop the connection, NM sends a SIGTERM to pppd to get disconnected.
Once pppd performs its own disconnection sequence it exits, and once
pppd exits, NM will tell MM to handle the disconnection.
Now, if pppd doesn't re-set CLOCAL before exiting, there will be some
time (between pppd exit and MM re-setting CLOCAL) where CLOCAL is unset
and modem control lines are in effect. If the kernel gets the DCD input
control line to low during that time, it may end up launching a full
hungup of the device, see Linux kernel's cdc-acm driver snippet:
case USB_CDC_NOTIFY_SERIAL_STATE:
newctrl = get_unaligned_le16(data);
if (!acm->clocal && (acm->ctrlin & ~newctrl & ACM_CTRL_DCD)) {
dev_dbg(&acm->control->dev, "%s - calling hangup\n", __func__);
tty_port_tty_hangup(&acm->port, false);
}
So, rather than exiting with CLOCAL unset, make sure it is always
re-set. It currently was only being re-set in non-hungup cases if there
was a disconnection script provided.
Is this the correct approach to solve the issue? Or am I missing something?
One note regarding the patch.... I left the:
stop_charshunt(NULL, 0);
call also to be run only if disconnect_script != NULL, not sure if that
should also be changed so that it is executed regardless of whether we
have a disconnect script or not.
Cheers!
--
Aleksander
>From feaca02edf036ae8b452459b36835aa0edeb7f10 Mon Sep 17 00:00:00 2001
From: Aleksander Morgado <aleksander@xxxxxxxxxxxxx>
Date: Wed, 23 Apr 2014 11:51:35 +0200
Subject: [PATCH] pppd: ensure CLOCAL is re-set upon disconnection
pppd will by default unset CLOCAL in the tty termios when connected, in order to
use modem control lines. But when pppd is stopped with SIGTERM, CLOCAL is not
being re-set in the tty.
Some background...
NetworkManager and ModemManager use pppd to setup the mobile broadband modem
connections. Once MM sets the tty in connected mode, NM launches pppd in the tty
and the session is established. When the user requests to stop the connection,
NM sends a SIGTERM to pppd to get disconnected. Once pppd performs its own
disconnection sequence it exits, and once pppd exits, NM will tell MM to handle
the disconnection.
Now, if pppd doesn't re-set CLOCAL before exiting, there will be some time
(between pppd exit and MM re-setting CLOCAL) where CLOCAL is unset and modem
control lines are in effect. If the kernel gets the DCD input control line
to low during that time, it may end up launching a full hungup of the device,
see Linux kernel's cdc-acm driver snippet:
case USB_CDC_NOTIFY_SERIAL_STATE:
newctrl = get_unaligned_le16(data);
if (!acm->clocal && (acm->ctrlin & ~newctrl & ACM_CTRL_DCD)) {
dev_dbg(&acm->control->dev, "%s - calling hangup\n", __func__);
tty_port_tty_hangup(&acm->port, false);
}
So, rather than exiting with CLOCAL unset, make sure it is always re-set. It
currently was only being re-set in non-hungup cases if there was a
disconnection script provided.
---
pppd/tty.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/pppd/tty.c b/pppd/tty.c
index d571b11..67d3b85 100644
--- a/pppd/tty.c
+++ b/pppd/tty.c
@@ -771,10 +771,13 @@ int connect_tty()
void disconnect_tty()
{
- if (disconnect_script == NULL || hungup)
+ if (hungup)
return;
if (real_ttyfd >= 0)
set_up_tty(real_ttyfd, 1);
+
+ if (disconnect_script == NULL)
+ return;
if (device_script(disconnect_script, ttyfd, ttyfd, 0) < 0) {
warn("disconnect script failed");
} else {
--
1.9.2
