Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 6 Nov 2017 11:16:46 +0100
From:   Steffen Klassert <steffen.klassert@secunet.com>
To:     Florian Westphal <fw@strlen.de>
CC:     syzbot 
        <bot+19b21aa652248382e2b8cbb81fa1cdc03b4bda01@syzkaller.appspotmail.com>,
        <davem@davemloft.net>, <herbert@gondor.apana.org.au>,
        <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
        <syzkaller-bugs@googlegroups.com>, <thomas.egerer@secunet.com>
Subject: Re: KASAN: stack-out-of-bounds Read in xfrm_state_find (2)
Message-ID: <20171106101646.GG23855@secunet.com>
References: <20171101220608.GA9424@breakpoint.cc>
 <20171102103237.GL11292@secunet.com>
 <20171102122528.GB9424@breakpoint.cc>
 <20171103121012.GA23855@secunet.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20171103121012.GA23855@secunet.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
X-G-Data-MailSecurity-for-Exchange-Sender: 23
X-G-Data-MailSecurity-for-Exchange-Server: d65e63f7-5c15-413f-8f63-c0d707471c93
X-EXCLAIMER-MD-CONFIG: 2c86f778-e09b-4440-8b15-867914633a10
X-G-Data-MailSecurity-for-Exchange-Guid: 6FF82398-F0AA-418B-982E-C953C4F254A8
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Nov 03, 2017 at 01:10:12PM +0100, Steffen Klassert wrote:
> On Thu, Nov 02, 2017 at 01:25:28PM +0100, Florian Westphal wrote:
> > Steffen Klassert <steffen.klassert@secunet.com> wrote:
> > 
> > > I'd propose to use the addresses from the template unconditionally,
> > > like the (untested) patch below does.
> > > 
> > > Unfortunalely the reproducer does not work with my config,
> > > sendto returns EAGAIN. Could anybody try this patch?
> > 
> > The reproducer no longer causes KASAN spew with your patch,
> > but i don't have a test case that actually creates/uses a tunnel.
> 
> The patch passed my standard tests, so I tend apply it
> after a day in the ipsec/testing branch.

FYI: I've just applied the patch below to the ipsec tree.

Subject: [PATCH ipsec] xfrm: Fix stack-out-of-bounds read in xfrm_state_find.

When we do tunnel or beet mode, we pass saddr and daddr from the
template to xfrm_state_find(), this is ok. On transport mode,
we pass the addresses from the flowi, assuming that the IP
addresses (and address family) don't change during transformation.
This assumption is wrong in the IPv4 mapped IPv6 case, packet
is IPv4 and template is IPv6. Fix this by using the addresses
from the template unconditionally.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index a2e531b..6eb228a 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1361,36 +1361,29 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, const struct flowi *fl,
 	struct net *net = xp_net(policy);
 	int nx;
 	int i, error;
-	xfrm_address_t *daddr = xfrm_flowi_daddr(fl, family);
-	xfrm_address_t *saddr = xfrm_flowi_saddr(fl, family);
 	xfrm_address_t tmp;
 
 	for (nx = 0, i = 0; i < policy->xfrm_nr; i++) {
 		struct xfrm_state *x;
-		xfrm_address_t *remote = daddr;
-		xfrm_address_t *local  = saddr;
+		xfrm_address_t *local;
+		xfrm_address_t *remote;
 		struct xfrm_tmpl *tmpl = &policy->xfrm_vec[i];
 
-		if (tmpl->mode == XFRM_MODE_TUNNEL ||
-		    tmpl->mode == XFRM_MODE_BEET) {
-			remote = &tmpl->id.daddr;
-			local = &tmpl->saddr;
-			if (xfrm_addr_any(local, tmpl->encap_family)) {
-				error = xfrm_get_saddr(net, fl->flowi_oif,
-						       &tmp, remote,
-						       tmpl->encap_family, 0);
-				if (error)
-					goto fail;
-				local = &tmp;
-			}
+		remote = &tmpl->id.daddr;
+		local = &tmpl->saddr;
+		if (xfrm_addr_any(local, tmpl->encap_family)) {
+			error = xfrm_get_saddr(net, fl->flowi_oif,
+					       &tmp, remote,
+					       tmpl->encap_family, 0);
+			if (error)
+				goto fail;
+			local = &tmp;
 		}
 
 		x = xfrm_state_find(remote, local, fl, tmpl, policy, &error, family);
 
 		if (x && x->km.state == XFRM_STATE_VALID) {
 			xfrm[nx++] = x;
-			daddr = remote;
-			saddr = local;
 			continue;
 		}
 		if (x) {
-- 
2.7.4


From 1583046794150760815@xxx Fri Nov 03 12:11:09 +0000 2017
X-GM-THRID: 1582886692223881834
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread