* pull request (net): ipsec 2017-11-16
@ 2017-11-16 10:00 Steffen Klassert
2017-11-16 10:00 ` [PATCH 1/2] xfrm: Copy policy family in clone_policy Steffen Klassert
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Steffen Klassert @ 2017-11-16 10:00 UTC (permalink / raw
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
1) Copy policy family in clone_policy, otherwise this can
trigger a BUG_ON in af_key. From Herbert Xu.
2) Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
This added a regression with transport mode when no addresses
are configured on the policy template.
Both patches are stable candidates.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit b39545684a90ef3374abc0969d64c7bc540d128d:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-11-11 09:10:39 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
for you to fetch changes up to 94802151894d482e82c324edf2c658f8e6b96508:
Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find." (2017-11-15 06:42:28 +0100)
----------------------------------------------------------------
Herbert Xu (1):
xfrm: Copy policy family in clone_policy
Steffen Klassert (1):
Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
net/xfrm/xfrm_policy.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] xfrm: Copy policy family in clone_policy
2017-11-16 10:00 pull request (net): ipsec 2017-11-16 Steffen Klassert
@ 2017-11-16 10:00 ` Steffen Klassert
2017-11-16 10:00 ` [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find." Steffen Klassert
2017-11-16 13:34 ` pull request (net): ipsec 2017-11-16 David Miller
2 siblings, 0 replies; 11+ messages in thread
From: Steffen Klassert @ 2017-11-16 10:00 UTC (permalink / raw
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Herbert Xu <herbert@gondor.apana.org.au>
The syzbot found an ancient bug in the IPsec code. When we cloned
a socket policy (for example, for a child TCP socket derived from a
listening socket), we did not copy the family field. This results
in a live policy with a zero family field. This triggers a BUG_ON
check in the af_key code when the cloned policy is retrieved.
This patch fixes it by copying the family field over.
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 6eb228a..2a60938 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1306,6 +1306,7 @@ static struct xfrm_policy *clone_policy(const struct xfrm_policy *old, int dir)
newp->xfrm_nr = old->xfrm_nr;
newp->index = old->index;
newp->type = old->type;
+ newp->family = old->family;
memcpy(newp->xfrm_vec, old->xfrm_vec,
newp->xfrm_nr*sizeof(struct xfrm_tmpl));
spin_lock_bh(&net->xfrm.xfrm_policy_lock);
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2017-11-16 10:00 pull request (net): ipsec 2017-11-16 Steffen Klassert
2017-11-16 10:00 ` [PATCH 1/2] xfrm: Copy policy family in clone_policy Steffen Klassert
@ 2017-11-16 10:00 ` Steffen Klassert
2017-12-23 9:22 ` Steffen Klassert
2017-11-16 13:34 ` pull request (net): ipsec 2017-11-16 David Miller
2 siblings, 1 reply; 11+ messages in thread
From: Steffen Klassert @ 2017-11-16 10:00 UTC (permalink / raw
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
This commit breaks transport mode when the policy template
has widlcard addresses configured, so revert it.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 2a60938..6bc16bb 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1362,29 +1362,36 @@ 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 *local;
- xfrm_address_t *remote;
+ xfrm_address_t *remote = daddr;
+ xfrm_address_t *local = saddr;
struct xfrm_tmpl *tmpl = &policy->xfrm_vec[i];
- 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;
+ 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;
+ }
}
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: pull request (net): ipsec 2017-11-16
2017-11-16 10:00 pull request (net): ipsec 2017-11-16 Steffen Klassert
2017-11-16 10:00 ` [PATCH 1/2] xfrm: Copy policy family in clone_policy Steffen Klassert
2017-11-16 10:00 ` [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find." Steffen Klassert
@ 2017-11-16 13:34 ` David Miller
2 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2017-11-16 13:34 UTC (permalink / raw
To: steffen.klassert; +Cc: herbert, netdev
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 16 Nov 2017 11:00:38 +0100
> 1) Copy policy family in clone_policy, otherwise this can
> trigger a BUG_ON in af_key. From Herbert Xu.
>
> 2) Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
> This added a regression with transport mode when no addresses
> are configured on the policy template.
>
> Both patches are stable candidates.
>
> Please pull or let me know if there are problems.
Pulled, thanks Steffen.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2017-11-16 10:00 ` [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find." Steffen Klassert
@ 2017-12-23 9:22 ` Steffen Klassert
2017-12-23 15:56 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Steffen Klassert @ 2017-12-23 9:22 UTC (permalink / raw
To: David Miller; +Cc: Herbert Xu, Derek Robson, netdev
On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
>
> This commit breaks transport mode when the policy template
> has widlcard addresses configured, so revert it.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
David, can you please queue this one up for v4.14-stable?
Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
v4.14 is unusable for some people without this revert.
Thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2017-12-23 9:22 ` Steffen Klassert
@ 2017-12-23 15:56 ` David Miller
2017-12-23 16:09 ` Steffen Klassert
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2017-12-23 15:56 UTC (permalink / raw
To: steffen.klassert; +Cc: herbert, robsonde, netdev
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Sat, 23 Dec 2017 10:22:17 +0100
> On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
>> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
>>
>> This commit breaks transport mode when the policy template
>> has widlcard addresses configured, so revert it.
>>
>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>
> David, can you please queue this one up for v4.14-stable?
> Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
>
> v4.14 is unusable for some people without this revert.
Yes, but it adds back the stack out-of-bounds bug.
If I queue up the revert, I would also need to queue up whatever
follow-on you used to fix the out-of-bounds bug properly. Which
commit is that?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2017-12-23 15:56 ` David Miller
@ 2017-12-23 16:09 ` Steffen Klassert
2017-12-23 18:10 ` robsonde
2018-01-05 16:12 ` Nicolas Dichtel
0 siblings, 2 replies; 11+ messages in thread
From: Steffen Klassert @ 2017-12-23 16:09 UTC (permalink / raw
To: David Miller; +Cc: herbert, robsonde, netdev
On Sat, Dec 23, 2017 at 10:56:12AM -0500, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Sat, 23 Dec 2017 10:22:17 +0100
>
> > On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
> >> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
> >>
> >> This commit breaks transport mode when the policy template
> >> has widlcard addresses configured, so revert it.
> >>
> >> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> >
> > David, can you please queue this one up for v4.14-stable?
> > Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
> >
> > v4.14 is unusable for some people without this revert.
>
> Yes, but it adds back the stack out-of-bounds bug.
>
> If I queue up the revert, I would also need to queue up whatever
> follow-on you used to fix the out-of-bounds bug properly. Which
> commit is that?
This is commit ddc47e4404b58f03e98345398fb12d38fe291512
("xfrm: Fix stack-out-of-bounds read on socket policy lookup.")
It is included in the pull request for the net tree that
I sent yesterday. The patch looks save, but not so sure
if it should go directly to stable. These bugs reported by
the syzbot are usually quite subtile and I already broke
something when I tried to fix the original stack out-of-bounds
bug. So maybe we should wait until the v4.15 release before
backporting...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2017-12-23 16:09 ` Steffen Klassert
@ 2017-12-23 18:10 ` robsonde
2018-01-05 16:12 ` Nicolas Dichtel
1 sibling, 0 replies; 11+ messages in thread
From: robsonde @ 2017-12-23 18:10 UTC (permalink / raw
To: Steffen Klassert; +Cc: David Miller, herbert, netdev
> On 24/12/2017, at 5:09 AM, Steffen Klassert <steffen.klassert@secunet.com> wrote:
>
>> On Sat, Dec 23, 2017 at 10:56:12AM -0500, David Miller wrote:
>> From: Steffen Klassert <steffen.klassert@secunet.com>
>> Date: Sat, 23 Dec 2017 10:22:17 +0100
>>
>>>> On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
>>>> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
>>>>
>>>> This commit breaks transport mode when the policy template
>>>> has widlcard addresses configured, so revert it.
>>>>
>>>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>>>
>>> David, can you please queue this one up for v4.14-stable?
>>> Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
>>>
>>> v4.14 is unusable for some people without this revert.
>>
>> Yes, but it adds back the stack out-of-bounds bug.
>>
>> If I queue up the revert, I would also need to queue up whatever
>> follow-on you used to fix the out-of-bounds bug properly. Which
>> commit is that?
>
> This is commit ddc47e4404b58f03e98345398fb12d38fe291512
> ("xfrm: Fix stack-out-of-bounds read on socket policy lookup.")
>
> It is included in the pull request for the net tree that
> I sent yesterday. The patch looks save, but not so sure
> if it should go directly to stable. These bugs reported by
> the syzbot are usually quite subtile and I already broke
> something when I tried to fix the original stack out-of-bounds
> bug. So maybe we should wait until the v4.15 release before
> backporting...
>
At this time I cant build an IPSec VPN on a 4.14 kernel. 4.14 is LTS and so has been used as kernel choice for a prod system. This is a production fault.
My only choice is going back to a 4.13 or waiting for 4.15 or a custom build, which I am not keen on.
Unless the memory bug effects security and can be exploited, then I ask that the revert be back ported to 4.14
Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2017-12-23 16:09 ` Steffen Klassert
2017-12-23 18:10 ` robsonde
@ 2018-01-05 16:12 ` Nicolas Dichtel
2018-01-05 17:17 ` David Miller
1 sibling, 1 reply; 11+ messages in thread
From: Nicolas Dichtel @ 2018-01-05 16:12 UTC (permalink / raw
To: Steffen Klassert, David Miller; +Cc: herbert, robsonde, netdev
Le 23/12/2017 à 17:09, Steffen Klassert a écrit :
> On Sat, Dec 23, 2017 at 10:56:12AM -0500, David Miller wrote:
>> From: Steffen Klassert <steffen.klassert@secunet.com>
>> Date: Sat, 23 Dec 2017 10:22:17 +0100
>>
>>> On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
>>>> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
>>>>
>>>> This commit breaks transport mode when the policy template
>>>> has widlcard addresses configured, so revert it.
>>>>
>>>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>>>
>>> David, can you please queue this one up for v4.14-stable?
>>> Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
>>>
>>> v4.14 is unusable for some people without this revert.
>>
>> Yes, but it adds back the stack out-of-bounds bug.
>>
>> If I queue up the revert, I would also need to queue up whatever
>> follow-on you used to fix the out-of-bounds bug properly. Which
>> commit is that?
>
> This is commit ddc47e4404b58f03e98345398fb12d38fe291512
> ("xfrm: Fix stack-out-of-bounds read on socket policy lookup.")
>
> It is included in the pull request for the net tree that
> I sent yesterday. The patch looks save, but not so sure
> if it should go directly to stable. These bugs reported by
> the syzbot are usually quite subtile and I already broke
> something when I tried to fix the original stack out-of-bounds
> bug. So maybe we should wait until the v4.15 release before
> backporting...
>
This patch is still missing in the 4.14 stable. Without it, some IPsec scenarii
are broken. Is there a plan to queue this patch for the 4.14 stable ?
Thank you,
Nicolas
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2018-01-05 16:12 ` Nicolas Dichtel
@ 2018-01-05 17:17 ` David Miller
2018-01-08 16:06 ` Nicolas Dichtel
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2018-01-05 17:17 UTC (permalink / raw
To: nicolas.dichtel; +Cc: steffen.klassert, herbert, robsonde, netdev
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 5 Jan 2018 17:12:59 +0100
> Le 23/12/2017 à 17:09, Steffen Klassert a écrit :
>> On Sat, Dec 23, 2017 at 10:56:12AM -0500, David Miller wrote:
>>> From: Steffen Klassert <steffen.klassert@secunet.com>
>>> Date: Sat, 23 Dec 2017 10:22:17 +0100
>>>
>>>> On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
>>>>> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
>>>>>
>>>>> This commit breaks transport mode when the policy template
>>>>> has widlcard addresses configured, so revert it.
>>>>>
>>>>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>>>>
>>>> David, can you please queue this one up for v4.14-stable?
>>>> Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
>>>>
>>>> v4.14 is unusable for some people without this revert.
>>>
>>> Yes, but it adds back the stack out-of-bounds bug.
>>>
>>> If I queue up the revert, I would also need to queue up whatever
>>> follow-on you used to fix the out-of-bounds bug properly. Which
>>> commit is that?
>>
>> This is commit ddc47e4404b58f03e98345398fb12d38fe291512
>> ("xfrm: Fix stack-out-of-bounds read on socket policy lookup.")
>>
>> It is included in the pull request for the net tree that
>> I sent yesterday. The patch looks save, but not so sure
>> if it should go directly to stable. These bugs reported by
>> the syzbot are usually quite subtile and I already broke
>> something when I tried to fix the original stack out-of-bounds
>> bug. So maybe we should wait until the v4.15 release before
>> backporting...
>>
> This patch is still missing in the 4.14 stable. Without it, some IPsec scenarii
> are broken. Is there a plan to queue this patch for the 4.14 stable ?
I will in my next batch of stable submissions.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
2018-01-05 17:17 ` David Miller
@ 2018-01-08 16:06 ` Nicolas Dichtel
0 siblings, 0 replies; 11+ messages in thread
From: Nicolas Dichtel @ 2018-01-08 16:06 UTC (permalink / raw
To: David Miller; +Cc: steffen.klassert, herbert, robsonde, netdev
Le 05/01/2018 à 18:17, David Miller a écrit :
[snip]
> I will in my next batch of stable submissions.
>
Thank you!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-01-08 16:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-16 10:00 pull request (net): ipsec 2017-11-16 Steffen Klassert
2017-11-16 10:00 ` [PATCH 1/2] xfrm: Copy policy family in clone_policy Steffen Klassert
2017-11-16 10:00 ` [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find." Steffen Klassert
2017-12-23 9:22 ` Steffen Klassert
2017-12-23 15:56 ` David Miller
2017-12-23 16:09 ` Steffen Klassert
2017-12-23 18:10 ` robsonde
2018-01-05 16:12 ` Nicolas Dichtel
2018-01-05 17:17 ` David Miller
2018-01-08 16:06 ` Nicolas Dichtel
2017-11-16 13:34 ` pull request (net): ipsec 2017-11-16 David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.