xl2tpd crash when tearing down L2TP/IPSec VPN connection

Bug #1677990 reported by Frode Nordahl
78
This bug affects 4 people
Affects Status Importance Assigned to Milestone
xl2tpd (Debian)
Fix Released
Unknown
xl2tpd (Ubuntu)
Fix Released
Medium
Unassigned
Xenial
Fix Released
Medium
Frode Nordahl
Yakkety
Fix Released
Medium
Frode Nordahl

Bug Description

[Impact]

 * xl2tpd crash with segmentation fault when disconnecting from L2TP/IPSec VPN

 * pppd processes never reaped, user will have to manually intervene to clean up

 * this will be a major annoyance for our users and I suggest we add this update to the stable release.

 * the proposed debdiff fixes this problem by patching a NULL-pointer de-reference in the upstream code.

[Test Case]

 * Set up L2TP/IPSec VPN server
   1. create a VM on your computer and install Ubuntu Xenial on it (must be VM, IPSec won't work in LXC)
   2. sudo apt install xl2tpd libssl-dev
   3. get and run this script: https://github.com/philpl/setup-strong-strongswan

 * Set up L2TP/IPSec VPN client
   1. sudo add-apt-repository ppa:nm-l2tp/network-manager-l2tp
       sudo apt update
       sudo apt install network-manager-l2tp
   2. sudo service xl2tpd stop (https://github.com/nm-l2tp/network-manager-l2tp/issues/38)
   3. Configure L2TP/IPSec VPN using Network Manager GUI and point it to the IP of your VM
   4. Connect
   5. Disconnect
   6. Observe that you see xl2tpd SIGSEGV in dmesg and that pppd is still running.

[Regression Potential]

 * The patch contains a check for NULL before de-referencing a pointer during cleanup. The same code has been tested for quite some time in the upstream 1.3.8 release that is available in Z and AA.

 * Patch already in Debian upstream for quite some time as well :
   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760602

[Original bug description]

Ubuntu Xenial

xl2tpd[20221]: segfault at 188 ip 000000000040bd08 sp 00007ffd8b6546b0 error 4 in xl2tpd[400000+1b000]

Core was generated by `/usr/sbin/xl2tpd -D -c /var/run/nm-xl2tpd.conf.20135 -C /var/run/nm-xl2tpd_l2tp'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000000000040bd08 in destroy_call (c=0x171d110) at call.c:420
420 call.c: No such file or directory.
(gdb) bt
#0 0x000000000040bd08 in destroy_call (c=0x171d110) at call.c:420
#1 0x000000000040bf90 in call_close (c=<optimized out>) at call.c:358
#2 0x000000000040c155 in call_close (c=0x171cb40) at call.c:335
#3 0x00000000004023d6 in death_handler (signal=signal@entry=15)
    at xl2tpd.c:294
#4 0x00000000004024bf in process_signal () at xl2tpd.c:338
#5 0x000000000040d016 in network_thread () at network.c:455
#6 0x0000000000401b96 in main (argc=<optimized out>, argv=<optimized out>)
    at xl2tpd.c:1557
(gdb) print *c
$1 = {lbit = 0, seq_reqd = 0, tx_pkts = 0, rx_pkts = 0, tx_bytes = 0,
  rx_bytes = 0, zlb_xmit = 0x0, prx = 0, state = 12, frame = 1, next = 0x0,
  debug = 0, msgtype = -1, ourcid = 106, cid = 10304, qcid = -1, bearer = -1,
  serno = 1, addr = 0, txspeed = 0, rxspeed = 0, ppd = 0, physchan = -1,
  dialed = '\000' <repeats 119 times>, dialing = '\000' <repeats 119 times>,
  subaddy = '\000' <repeats 119 times>, needclose = 0, closing = -1,
  container = 0x171c6a0, fd = -1, oldptyconf = 0x171d460, die = 0, nego = 0,
  pppd = 20222, result = -1, error = -1, fbit = 0, ourfbit = 0, cnu = 0,
  pnu = 0, errormsg = '\000' <repeats 119 times>, lastsent = {tv_sec = 0,
    tv_usec = 0}, data_seq_num = 0, data_rec_seq_num = 0, closeSs = 0,
  pLr = -1, lns = 0x0, lac = 0x171d4d0, dial_no = '\000' <repeats 127 times>}
(gdb) print c->lns
$2 = (struct lns *) 0x0
(gdb)

This is a NULL pointer de-reference and is fixed in this commit:
https://github.com/xelerance/xl2tpd/commit/a193e02c741168a9b9072b523f2d6faf14a049da

Frode Nordahl (fnordahl)
tags: added: sts sts-sru-needed
Revision history for this message
Frode Nordahl (fnordahl) wrote :
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in xl2tpd (Ubuntu):
status: New → Confirmed
Revision history for this message
Frode Nordahl (fnordahl) wrote :

Affects Xenial and Yakkety. Allready fixed in Zesty.

Mathew Hodson (mhodson)
Changed in xl2tpd (Ubuntu):
importance: Undecided → Medium
Frode Nordahl (fnordahl)
tags: added: sts-sponsor
Revision history for this message
Ilis (ilis) wrote :

xl2tpd version: xl2tpd-1.3.6
Distributor ID: LinuxMint
Release: 18.1

Frode Nordahl (fnordahl)
description: updated
Revision history for this message
Frode Nordahl (fnordahl) wrote :
Eric Desrochers (slashd)
Changed in xl2tpd (Ubuntu Xenial):
status: New → Confirmed
Changed in xl2tpd (Ubuntu Yakkety):
status: New → Incomplete
status: Incomplete → Confirmed
importance: Undecided → Medium
Changed in xl2tpd (Ubuntu Xenial):
importance: Undecided → Medium
assignee: nobody → Frode Nordahl (fnordahl)
Changed in xl2tpd (Ubuntu Yakkety):
assignee: nobody → Frode Nordahl (fnordahl)
Changed in xl2tpd (Ubuntu Xenial):
status: Confirmed → In Progress
Changed in xl2tpd (Ubuntu Yakkety):
status: Confirmed → In Progress
Changed in xl2tpd (Ubuntu):
status: Confirmed → Fix Released
Frode Nordahl (fnordahl)
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :
description: updated
Changed in xl2tpd (Debian):
status: Unknown → Fix Released
Eric Desrochers (slashd)
tags: added: sts-sru-done
removed: sts-sponsor sts-sru-needed
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Frode, or anyone else affected,

Accepted xl2tpd into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/xl2tpd/1.3.6+dfsg-4ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in xl2tpd (Ubuntu Yakkety):
status: In Progress → Fix Committed
tags: added: verification-needed
Changed in xl2tpd (Ubuntu Xenial):
status: In Progress → Fix Committed
Revision history for this message
Brian Murray (brian-murray) wrote :

Hello Frode, or anyone else affected,

Accepted xl2tpd into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/xl2tpd/1.3.6+dfsg-4ubuntu0.16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Revision history for this message
Brian Murray (brian-murray) wrote :

Unsubscribing sponsors since there is nothing left to upload.

Revision history for this message
Douglas Kosovic (dkosovic) wrote :

Hi Brian,

I tested xl2tpd_1.3.6+dfsg-4ubuntu0.16.04.1_amd64.deb on xenial with NetworkManager-l2tp and I'm no longer able to reproduce the xl2tpd segmentation fault, nor is there any orphaned pppd process (which used to happen after the parent xl2tpd process crashed)

Similarly with xl2tpd_1.3.6+dfsg-4ubuntu1_amd64.deb on yakkety.

Eric Desrochers (slashd)
tags: added: verification-done-xenial verification-done-yakkety
removed: verification-needed
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for xl2tpd has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xl2tpd - 1.3.6+dfsg-4ubuntu0.16.04.1

---------------
xl2tpd (1.3.6+dfsg-4ubuntu0.16.04.1) xenial; urgency=medium

  * d/p/xl2tpd-call-c-nullptr-dereference.patch (LP: #1677990)
    Backport fix for NULL-pointer dereference. (Closes: #760602)

 -- Frode Nordahl <email address hidden> Wed, 10 May 2017 14:16:17 +0000

Changed in xl2tpd (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xl2tpd - 1.3.6+dfsg-4ubuntu1

---------------
xl2tpd (1.3.6+dfsg-4ubuntu1) yakkety; urgency=medium

  * d/p/xl2tpd-call-c-nullptr-dereference.patch (LP: #1677990)
    Backport fix for NULL-pointer dereference. (Closes: #760602)

 -- Frode Nordahl <email address hidden> Wed, 10 May 2017 14:16:17 +0000

Changed in xl2tpd (Ubuntu Yakkety):
status: Fix Committed → Fix Released
Ilis (ilis)
no longer affects: linuxmint
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.