lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 18 Jan 2022 16:03:33 -0800 From: Yan Yan <evitayan@...gle.com> To: Steffen Klassert <steffen.klassert@...unet.com> Cc: netdev@...r.kernel.org, Herbert Xu <herbert@...dor.apana.org.au>, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Lorenzo Colitti <lorenzo@...gle.com>, Maciej Żenczykowski <maze@...gle.com>, nharold@...glel.com, benedictwong@...glel.com Subject: Re: [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes Thanks Steffen, I will pull out xfrm_init_state() as you suggested. On Tue, Jan 11, 2022 at 11:57 PM Steffen Klassert <steffen.klassert@...unet.com> wrote: > > On Wed, Jan 05, 2022 at 04:52:51PM -0800, Yan Yan wrote: > > ... > > > -static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig, > > - struct xfrm_encap_tmpl *encap) > > +static struct xfrm_state *xfrm_state_clone1(struct xfrm_state *orig, > > + struct xfrm_encap_tmpl *encap) > > { > > struct net *net = xs_net(orig); > > struct xfrm_state *x = xfrm_state_alloc(net); > > @@ -1579,8 +1579,20 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig, > > memcpy(&x->mark, &orig->mark, sizeof(x->mark)); > > memcpy(&x->props.smark, &orig->props.smark, sizeof(x->props.smark)); > > > > - if (xfrm_init_state(x) < 0) > > - goto error; > > + return x; > > + > > + error: > > + xfrm_state_put(x); > > +out: > > + return NULL; > > +} > > + > > +static int xfrm_state_clone2(struct xfrm_state *orig, struct xfrm_state *x) > > I'm not a frind of numbering function names, this just invites to > create xfrm_state_clone3 :) > > > +{ > > + int err = xfrm_init_state(x); > > + > > + if (err < 0) > > + return err; > > > > x->props.flags = orig->props.flags; > > x->props.extra_flags = orig->props.extra_flags; > > @@ -1595,12 +1607,7 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig, > > x->replay = orig->replay; > > x->preplay = orig->preplay; > > > > - return x; > > - > > - error: > > - xfrm_state_put(x); > > -out: > > - return NULL; > > + return 0; > > } > > > > struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net, > > @@ -1661,10 +1668,14 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x, > > { > > struct xfrm_state *xc; > > > > - xc = xfrm_state_clone(x, encap); > > + xc = xfrm_state_clone1(x, encap); > > if (!xc) > > return NULL; > > > > + xc->props.family = m->new_family; > > + if (xfrm_state_clone2(x, xc) < 0) > > + goto error; > > xfrm_state_migrate() is the only function that calls xfrm_state_clone(). > Wouldn't it be better to move xfrm_init_state() out of xfrm_state_clone() > and call it afterwards? > > This would also fix the replay window initialization on ESN because > currently x->props.flags (which holds XFRM_STATE_ESN) is initialized > after xfrm_init_state(). > -- -- Best, Yan
Powered by blists - more mailing lists