NetBSD Problem Report #53189
From martin@duskware.de Mon Apr 16 15:17:39 2018
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id A4B997A19F
for <gnats-bugs@gnats.NetBSD.org>; Mon, 16 Apr 2018 15:17:39 +0000 (UTC)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: reproducable kernel assertion
X-Send-Pr-Version: 3.95
>Number: 53189
>Category: kern
>Synopsis: reproducable kernel assertion
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: kern-bug-people
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Apr 16 15:20:00 +0000 2018
>Closed-Date: Thu May 03 08:04:13 +0000 2018
>Last-Modified: Thu May 03 08:04:13 +0000 2018
>Originator: Martin Husemann
>Release: NetBSD 8.99.14
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD space-truckin.duskware.de 8.99.14 NetBSD 8.99.14 (SUNXI) #35: Mon Apr 16 11:06:40 CEST 2018 martin@night-owl.duskware.de:/usr/src/sys/arch/evbarm/compile/SUNXI evbarm
Architecture: earmv7hfeb
Machine: evbarm
>Description:
Running this test case:
fs/psshfs/t_psshfs (587/738): 4 test cases
inode_nos:
reproducably panics my earmv7hfeb and sparc64 machines (but alpha seems
to survive).
[ 939.4386985] panic: m_verify_packet: M_PKTHDR set on secondary mbuf
[ 939.4510184] cpu0: Begin traceback...
[ 939.4510184] 0x9a811dcc: netbsd:db_panic+0x14
[ 939.4587925] 0x9a811de4: netbsd:vpanic+0x194
[ 939.4587925] 0x9a811dfc: netbsd:snprintf
[ 939.4587925] 0x9a811e2c: netbsd:m_verify_packet+0xc8
[ 939.4704970] 0x9a811f14: netbsd:ip6_input+0x790
[ 939.4704970] 0x9a811f4c: netbsd:ip6intr+0x88
[ 939.4805137] 0x9a811fac: netbsd:softint_dispatch+0x114
[ 939.4805137] 0x9d879dbc: netbsd:softint_switch+0x58
[ 939.4909527] 0x9d879dec: netbsd:splx+0xc8
[ 939.4909527] 0x9d879e84: netbsd:sosend+0x780
[ 939.5005167] 0x9d879eac: netbsd:soo_write+0x3c
[ 939.5005167] 0x9d879f04: netbsd:dofilewrite+0x98
[ 939.5114988] 0x9d879f34: netbsd:sys_write+0x70
[ 939.5114988] 0x9d879fac: netbsd:syscall+0x164
>How-To-Repeat:
cd /usr/tests/fs/psshfs && atf-run
>Fix:
n/a
>Release-Note:
>Audit-Trail:
From: "Maxime Villard" <maxv@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: src/sys/kern
Date: Mon, 16 Apr 2018 19:19:51 +0000
Module Name: src
Committed By: maxv
Date: Mon Apr 16 19:19:51 UTC 2018
Modified Files:
src/sys/kern: uipc_mbuf.c
Log Message:
Disable the M_PKTHDR check for now. It causes PR/53189 (which is also
reproducible on i386).
It seems that someone is giving looutput a malformed chain.
To generate a diff of this commit:
cvs rdiff -u -r1.188 -r1.189 src/sys/kern/uipc_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Maxime Villard" <maxv@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: src/sys/netipsec
Date: Tue, 17 Apr 2018 06:23:30 +0000
Module Name: src
Committed By: maxv
Date: Tue Apr 17 06:23:30 UTC 2018
Modified Files:
src/sys/netipsec: ipsec_mbuf.c
Log Message:
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-8] src/sys/netipsec
Date: Tue, 17 Apr 2018 15:06:20 +0000
Module Name: src
Committed By: martin
Date: Tue Apr 17 15:06:20 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-8]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #773):
sys/netipsec/ipsec_mbuf.c: revision 1.23,1.24
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.16 -r1.16.2.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-7] src/sys/netipsec
Date: Tue, 17 Apr 2018 15:10:53 +0000
Module Name: src
Committed By: martin
Date: Tue Apr 17 15:10:53 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-7]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #1599):
sys/netipsec/ipsec_mbuf.c: revision 1.23,1.24 (via patch)
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.30.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->analyzed
State-Changed-By: maxv@NetBSD.org
State-Changed-When: Tue, 17 Apr 2018 15:16:56 +0000
State-Changed-Why:
There are many places that actually put M_PKTHDR on secondary mbufs. In
your case, it was in tcp_build_datapkt().
I've disabled the check, and it will probably remain disabled. It is
difficult to know whether such chains can trigger bugs; they certainly
can, but only few places may be affected. So far, I identified only one
(IPsec), and I fixed it.
For now I believe the assumption should be:
"M_PKTHDR should not be set on secondary mbufs. However,
everybody should be ready to handle this case."
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-7-1] src/sys/netipsec
Date: Tue, 17 Apr 2018 15:38:12 +0000
Module Name: src
Committed By: martin
Date: Tue Apr 17 15:38:12 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-7-1]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #1599):
sys/netipsec/ipsec_mbuf.c: revision 1.23,1.24 (via patch)
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.42.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-7-0] src/sys/netipsec
Date: Tue, 17 Apr 2018 15:38:57 +0000
Module Name: src
Committed By: martin
Date: Tue Apr 17 15:38:57 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-7-0]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #1599):
sys/netipsec/ipsec_mbuf.c: revision 1.23,1.24 (via patch)
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.34.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-6] src/sys/netipsec
Date: Wed, 18 Apr 2018 06:59:10 +0000
Module Name: src
Committed By: msaitoh
Date: Wed Apr 18 06:59:10 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-6]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #1545):
sys/netipsec/ipsec_mbuf.c: revision 1.23
sys/netipsec/ipsec_mbuf.c: revision 1.24
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.10.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-6-1] src/sys/netipsec
Date: Wed, 18 Apr 2018 07:17:24 +0000
Module Name: src
Committed By: msaitoh
Date: Wed Apr 18 07:17:24 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-6-1]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #1545):
sys/netipsec/ipsec_mbuf.c: revision 1.23
sys/netipsec/ipsec_mbuf.c: revision 1.24
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.24.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53189 CVS commit: [netbsd-6-0] src/sys/netipsec
Date: Wed, 18 Apr 2018 07:17:48 +0000
Module Name: src
Committed By: msaitoh
Date: Wed Apr 18 07:17:48 UTC 2018
Modified Files:
src/sys/netipsec [netbsd-6-0]: ipsec_mbuf.c
Log Message:
Pull up following revision(s) (requested by maxv in ticket #1545):
sys/netipsec/ipsec_mbuf.c: revision 1.23
sys/netipsec/ipsec_mbuf.c: revision 1.24
Don't assume M_PKTHDR is set only on the first mbuf of the chain. It
should, but it looks like there are several places that can put M_PKTHDR
on secondary mbufs (PR/53189), so drop this assumption right now to
prevent further bugs.
The check is replaced by (m1 != m), which is equivalent to the previous
code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
m_adj().
Fix a pretty bad mistake, that has always been there.
m_adj(m1, -(m1->m_len - roff));
if (m1 != m)
m->m_pkthdr.len -= (m1->m_len - roff);
This is wrong: m_adj will modify m1->m_len, so we're using a wrong value
when manually adjusting m->m_pkthdr.len.
Because of that, it is possible to exploit the attack I described in
uipc_mbuf.c::rev1.182. The exploit is more complicated, but works 100%
reliably.
To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.12.16.1 src/sys/netipsec/ipsec_mbuf.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: analyzed->closed
State-Changed-By: maxv@NetBSD.org
State-Changed-When: Thu, 03 May 2018 08:04:13 +0000
State-Changed-Why:
Close this PR. I disabled the KASSERT, because several places put M_PKTHDR
on secondary mbufs, and we're in such a state that it is not possible to
guarantee that this KASSERT is not fired on a regular basis.
So we must assume that the assumption does not hold, even though it would
be theoretically consistent if it did. I added a comment in uipc_mbuf.c.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home