| Message ID | 1510826440-19452-3-git-send-email-steffen.klassert@secunet.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/2] xfrm: Copy policy family in clone_policy | expand |
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!
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?
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...
> 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
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
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.
Le 05/01/2018 à 18:17, David Miller a écrit : [snip] > I will in my next batch of stable submissions. > Thank you!
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) {
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(-)