Bug 146082 - [ng_l2tp] a false invaliant check was performed in ng_l2tp_seq_check()
Summary: [ng_l2tp] a false invaliant check was performed in ng_l2tp_seq_check()
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 7.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Gleb Smirnoff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-27 10:30 UTC by Yoshihiko Sarumaru
Modified: 2014-01-22 09:30 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yoshihiko Sarumaru 2010-04-27 10:30:08 UTC
Sometimes the same panic was observed in my FreeBSD IPv6 box + L2TP tunnel.

In ng_l2tp_seq_check(), peer_nack was ensured positive value by:

/* Compare sequence numbers using circular math */
#define L2TP_SEQ_DIFF(x, y)     ((int)((int16_t)(x) - (int16_t)(y)))

        int self_unack, peer_unack;
        peer_unack = L2TP_SEQ_DIFF(seq->ns, seq->rack);
        CHECK(peer_unack >= 0);
(picked up from src/sys/netgraph/ng_l2tp.c)

While (seq->ns >= 32768 && seq->rack < 32768) is true, 
this MACRO introduce a false value, and cause a panic.
(ns and rack are uint16_t variable)

NOTE: this function is effective when INVARIANT option was set.

backtrace:
#0  doadump () at pcpu.h:196
#1  0xc07ffbce in boot (howto=260) at /usr/src/sys/kern/kern_shutdown.c:418
#2  0xc07ffea2 in panic (fmt=Variable "fmt" is not available.
) at /usr/src/sys/kern/kern_shutdown.c:574
#3  0xc473baf0 in ng_l2tp_seq_check (seq=0xc41fa468)
    at /usr/src/sys/modules/netgraph/l2tp/../../../netgraph/ng_l2tp.c:1588
#4  0xc473c301 in ng_l2tp_rcvdata_ctrl (hook=0xc4614e80, item=0xc4739270)
    at /usr/src/sys/modules/netgraph/l2tp/../../../netgraph/ng_l2tp.c:1088
#5  0xc46ba0af in ng_apply_item (node=0xc4614d80, item=0xc4739270, rw=0)
    at /usr/src/sys/modules/netgraph/netgraph/../../../netgraph/ng_base.c:2336
#6  0xc46bb701 in ng_snd_item (item=0xc4739270, flags=Variable "flags" is not av
ailable.
)
    at /usr/src/sys/modules/netgraph/netgraph/../../../netgraph/ng_base.c:2254
#7  0xc46b2ad5 in ngd_send (so=0xc4738000, flags=0, m=0xc452a600,
    addr=0xc445e880, control=0x0, td=0xc46af460)
    at /usr/src/sys/modules/netgraph/socket/../../../netgraph/ng_socket.c:445
#8  0xc085e68d in sosend_generic (so=0xc4738000, addr=0xc445e880,
    uio=0xe67f2be8, top=0xc452a600, control=0x0, flags=0, td=0xc46af460)
    at /usr/src/sys/kern/uipc_socket.c:1246
#9  0xc085a89f in sosend (so=0xc4738000, addr=0xc445e880, uio=0xe67f2be8,
    top=0x0, control=0x0, flags=0, td=0xc46af460)
    at /usr/src/sys/kern/uipc_socket.c:1288
#10 0xc0861d16 in kern_sendit (td=0xc46af460, s=10, mp=0xe67f2c64, flags=0,
    control=0x0, segflg=UIO_USERSPACE) at /usr/src/sys/kern/uipc_syscalls.c:805
#11 0xc0863f01 in sendit (td=0xc46af460, s=10, mp=0xe67f2c64, flags=0)
    at /usr/src/sys/kern/uipc_syscalls.c:742
#12 0xc0864018 in sendto (td=0xc46af460, uap=0xe67f2cfc)
    at /usr/src/sys/kern/uipc_syscalls.c:857
#13 0xc0af65b3 in syscall (frame=0xe67f2d38)
    at /usr/src/sys/i386/i386/trap.c:1090
#14 0xc0adad40 in Xint0x80_syscall () at /usr/src/sys/i386/i386/exception.s:255
---Type <return> to continue, or q <return> to quit---
#15 0x00000033 in ?? ()
Previous frame inner to this frame (corrupt stack?)

state of *seq:
(kgdb) print *seq
$4 = {ns = 32768, nr = 2, inproc = 0, rack = 32767, xack = 2, wmax = 128,
  cwnd = 128, ssth = 1, acks = 0, rexmits = 0, rack_timer = {c_links = {sle = {
        sle_next = 0x0}, tqe = {tqe_next = 0x0, tqe_prev = 0xd80934a8}},
    c_time = 1965926795, c_arg = 0xc4a0b270,
    c_func = 0xc46bb9b0 <ng_callout_trampoline>, c_mtx = 0x0, c_flags = 18},
  xack_timer = {c_links = {sle = {sle_next = 0x0}, tqe = {tqe_next = 0x0,
        tqe_prev = 0xd809ebd0}}, c_time = 16496, c_arg = 0x0,
    c_func = 0xc46bb9b0 <ng_callout_trampoline>, c_mtx = 0x0, c_flags = 16},
  xwin = {0xc452a600, 0x0 <repeats 127 times>}, mtx = {lock_object = {
      lo_name = 0xc473e16f "ng_l2tp", lo_type = 0xc473e16f "ng_l2tp",
      lo_flags = 16973824, lo_witness_data = {lod_list = {
          stqe_next = 0xc0cfc9f0}, lod_witness = 0xc0cfc9f0}},
    mtx_lock = 3295343714, mtx_recurse = 0}}

Fix: 

I'm not sure.
How-To-Repeat: make an L2TP tunnel with ng_l2tp, then send and receive packet to/from that
interface for a long time (1-3 months in my home). 

INVARIANT option is required for your kernel.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2010-05-08 15:47:14 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-net

Over to maintainer(s).
Comment 2 dfilter service freebsd_committer freebsd_triage 2014-01-03 12:07:02 UTC
Author: glebius
Date: Fri Jan  3 12:06:54 2014
New Revision: 260225
URL: http://svnweb.freebsd.org/changeset/base/260225

Log:
  Fix circular math macro.
  
  Submitted by:		Lutz Donnerhacke via Dmitry Luhtionov
  German lesson at:	http://lutz.donnerhacke.de/Blog/Der-Fluch-der-Stabilitaet
  PR:			146082

Modified:
  head/sys/netgraph/ng_l2tp.c

Modified: head/sys/netgraph/ng_l2tp.c
==============================================================================
--- head/sys/netgraph/ng_l2tp.c	Fri Jan  3 11:03:12 2014	(r260224)
+++ head/sys/netgraph/ng_l2tp.c	Fri Jan  3 12:06:54 2014	(r260225)
@@ -98,7 +98,7 @@ static MALLOC_DEFINE(M_NETGRAPH_L2TP, "n
 #define L2TP_ENABLE_DSEQ	1			/* enable data seq # */
 
 /* Compare sequence numbers using circular math */
-#define L2TP_SEQ_DIFF(x, y)	((int)((int16_t)(x) - (int16_t)(y)))
+#define L2TP_SEQ_DIFF(x, y)	((int16_t)((x) - (y)))
 
 #define SESSHASHSIZE		0x0020
 #define SESSHASH(x)		(((x) ^ ((x) >> 8)) & (SESSHASHSIZE - 1))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 3 Gleb Smirnoff freebsd_committer freebsd_triage 2014-01-03 12:07:15 UTC
State Changed
From-To: open->patched

Fixed in head. 


Comment 4 Gleb Smirnoff freebsd_committer freebsd_triage 2014-01-03 12:07:15 UTC
Responsible Changed
From-To: freebsd-net->glebius

Fixed in head.
Comment 5 Gleb Smirnoff freebsd_committer freebsd_triage 2014-01-22 09:20:43 UTC
State Changed
From-To: patched->closed

Fix merged to 9.2-STABLE and 10.0-STABLE.
Comment 6 dfilter service freebsd_committer freebsd_triage 2014-01-22 09:22:46 UTC
Author: glebius
Date: Wed Jan 22 09:22:39 2014
New Revision: 261009
URL: http://svnweb.freebsd.org/changeset/base/261009

Log:
  Merge 260225:
  
    Fix circular math macro.
  
  PR:	146082

Modified:
  stable/10/sys/netgraph/ng_l2tp.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/netgraph/ng_l2tp.c
==============================================================================
--- stable/10/sys/netgraph/ng_l2tp.c	Wed Jan 22 08:33:32 2014	(r261008)
+++ stable/10/sys/netgraph/ng_l2tp.c	Wed Jan 22 09:22:39 2014	(r261009)
@@ -98,7 +98,7 @@ static MALLOC_DEFINE(M_NETGRAPH_L2TP, "n
 #define L2TP_ENABLE_DSEQ	1			/* enable data seq # */
 
 /* Compare sequence numbers using circular math */
-#define L2TP_SEQ_DIFF(x, y)	((int)((int16_t)(x) - (int16_t)(y)))
+#define L2TP_SEQ_DIFF(x, y)	((int16_t)((x) - (y)))
 
 #define SESSHASHSIZE		0x0020
 #define SESSHASH(x)		(((x) ^ ((x) >> 8)) & (SESSHASHSIZE - 1))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 dfilter service freebsd_committer freebsd_triage 2014-01-22 09:23:38 UTC
Author: glebius
Date: Wed Jan 22 09:23:30 2014
New Revision: 261010
URL: http://svnweb.freebsd.org/changeset/base/261010

Log:
  Merge 260225:
  
    Fix circular math macro.
  
  PR:	146082

Modified:
  stable/9/sys/netgraph/ng_l2tp.c
Directory Properties:
  stable/9/sys/   (props changed)

Modified: stable/9/sys/netgraph/ng_l2tp.c
==============================================================================
--- stable/9/sys/netgraph/ng_l2tp.c	Wed Jan 22 09:22:39 2014	(r261009)
+++ stable/9/sys/netgraph/ng_l2tp.c	Wed Jan 22 09:23:30 2014	(r261010)
@@ -98,7 +98,7 @@ static MALLOC_DEFINE(M_NETGRAPH_L2TP, "n
 #define L2TP_ENABLE_DSEQ	1			/* enable data seq # */
 
 /* Compare sequence numbers using circular math */
-#define L2TP_SEQ_DIFF(x, y)	((int)((int16_t)(x) - (int16_t)(y)))
+#define L2TP_SEQ_DIFF(x, y)	((int16_t)((x) - (y)))
 
 #define SESSHASHSIZE		0x0020
 #define SESSHASH(x)		(((x) ^ ((x) >> 8)) & (SESSHASHSIZE - 1))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"