* [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector
@ 2008-11-04 10:24 Arnaud Ebalard
2008-11-04 11:24 ` Herbert Xu
0 siblings, 1 reply; 5+ messages in thread
From: Arnaud Ebalard @ 2008-11-04 10:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Herbert Xu, Andreas Steffen, Martin Willi
Hi,
While adding MIGRATE support to strongSwan, Andreas Steffen noticed that
the selectors provided in XFRM_MSG_ACQUIRE have their family field
uninitialized (those in MIGRATE do have their family set).
Looking at the code, this is because the af-specific init_tempsel()
(called via afinfo->init_tempsel() in xfrm_init_tempsel()) do not set
the value.
Even if current apps probably do not rely on it, is there any argument
for not doing it or is it just an omission?
The patch below is more for discussion than anything else.
Cheers,
a+
Reported-by: Andreas Steffen <andreas.steffen@strongswan.org>
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
net/ipv4/xfrm4_state.c | 1 +
net/ipv6/xfrm6_state.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
index 07735ed..55dc6be 100644
--- a/net/ipv4/xfrm4_state.c
+++ b/net/ipv4/xfrm4_state.c
@@ -33,6 +33,7 @@ __xfrm4_init_tempsel(struct xfrm_state *x, struct flowi *fl,
x->sel.dport_mask = htons(0xffff);
x->sel.sport = xfrm_flowi_sport(fl);
x->sel.sport_mask = htons(0xffff);
+ x->sel.family = AF_INET;
x->sel.prefixlen_d = 32;
x->sel.prefixlen_s = 32;
x->sel.proto = fl->proto;
diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index 89884a4..60c78cf 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -34,6 +34,7 @@ __xfrm6_init_tempsel(struct xfrm_state *x, struct flowi *fl,
x->sel.dport_mask = htons(0xffff);
x->sel.sport = xfrm_flowi_sport(fl);
x->sel.sport_mask = htons(0xffff);
+ x->sel.family = AF_INET6;
x->sel.prefixlen_d = 128;
x->sel.prefixlen_s = 128;
x->sel.proto = fl->proto;
--
1.5.6.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector
2008-11-04 10:24 [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector Arnaud Ebalard
@ 2008-11-04 11:24 ` Herbert Xu
2008-11-04 11:46 ` Arnaud Ebalard
0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2008-11-04 11:24 UTC (permalink / raw)
To: Arnaud Ebalard
Cc: David Miller, netdev, Andreas Steffen, Martin Willi,
Kazunori MIYAZAWA
On Tue, Nov 04, 2008 at 11:24:51AM +0100, Arnaud Ebalard wrote:
> Hi,
>
> While adding MIGRATE support to strongSwan, Andreas Steffen noticed that
> the selectors provided in XFRM_MSG_ACQUIRE have their family field
> uninitialized (those in MIGRATE do have their family set).
>
> Looking at the code, this is because the af-specific init_tempsel()
> (called via afinfo->init_tempsel() in xfrm_init_tempsel()) do not set
> the value.
>
> Even if current apps probably do not rely on it, is there any argument
> for not doing it or is it just an omission?
>
> The patch below is more for discussion than anything else.
We should ask the MIP6 folks since this may affect them.
> Reported-by: Andreas Steffen <andreas.steffen@strongswan.org>
> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
> ---
> net/ipv4/xfrm4_state.c | 1 +
> net/ipv6/xfrm6_state.c | 1 +
> 2 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
> index 07735ed..55dc6be 100644
> --- a/net/ipv4/xfrm4_state.c
> +++ b/net/ipv4/xfrm4_state.c
> @@ -33,6 +33,7 @@ __xfrm4_init_tempsel(struct xfrm_state *x, struct flowi *fl,
> x->sel.dport_mask = htons(0xffff);
> x->sel.sport = xfrm_flowi_sport(fl);
> x->sel.sport_mask = htons(0xffff);
> + x->sel.family = AF_INET;
> x->sel.prefixlen_d = 32;
> x->sel.prefixlen_s = 32;
> x->sel.proto = fl->proto;
> diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
> index 89884a4..60c78cf 100644
> --- a/net/ipv6/xfrm6_state.c
> +++ b/net/ipv6/xfrm6_state.c
> @@ -34,6 +34,7 @@ __xfrm6_init_tempsel(struct xfrm_state *x, struct flowi *fl,
> x->sel.dport_mask = htons(0xffff);
> x->sel.sport = xfrm_flowi_sport(fl);
> x->sel.sport_mask = htons(0xffff);
> + x->sel.family = AF_INET6;
> x->sel.prefixlen_d = 128;
> x->sel.prefixlen_s = 128;
> x->sel.proto = fl->proto;
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector
2008-11-04 11:24 ` Herbert Xu
@ 2008-11-04 11:46 ` Arnaud Ebalard
2008-11-04 11:52 ` Herbert Xu
0 siblings, 1 reply; 5+ messages in thread
From: Arnaud Ebalard @ 2008-11-04 11:46 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, netdev, Andreas Steffen, Martin Willi,
Kazunori MIYAZAWA
Hi,
Herbert Xu <herbert@gondor.apana.org.au> writes:
> On Tue, Nov 04, 2008 at 11:24:51AM +0100, Arnaud Ebalard wrote:
>> Hi,
>>
>> While adding MIGRATE support to strongSwan, Andreas Steffen noticed that
>> the selectors provided in XFRM_MSG_ACQUIRE have their family field
>> uninitialized (those in MIGRATE do have their family set).
>>
>> Looking at the code, this is because the af-specific init_tempsel()
>> (called via afinfo->init_tempsel() in xfrm_init_tempsel()) do not set
>> the value.
>>
>> Even if current apps probably do not rely on it, is there any argument
>> for not doing it or is it just an omission?
>>
>> The patch below is more for discussion than anything else.
>
> We should ask the MIP6 folks since this may affect them.
Sorry Herbert, my initial comment was misleading: the family is not set
in the selectors provided in the *XFRM_MSG_ACQUIRE*, which is not MIPv6
related. I could check again, but I think the patch below will impact
all native key managers. Or did I miss something and there is a specific
reason why MIPv6 folks may be impacted?
>> Reported-by: Andreas Steffen <andreas.steffen@strongswan.org>
>> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
>> ---
>> net/ipv4/xfrm4_state.c | 1 +
>> net/ipv6/xfrm6_state.c | 1 +
>> 2 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
>> index 07735ed..55dc6be 100644
>> --- a/net/ipv4/xfrm4_state.c
>> +++ b/net/ipv4/xfrm4_state.c
>> @@ -33,6 +33,7 @@ __xfrm4_init_tempsel(struct xfrm_state *x, struct flowi *fl,
>> x->sel.dport_mask = htons(0xffff);
>> x->sel.sport = xfrm_flowi_sport(fl);
>> x->sel.sport_mask = htons(0xffff);
>> + x->sel.family = AF_INET;
>> x->sel.prefixlen_d = 32;
>> x->sel.prefixlen_s = 32;
>> x->sel.proto = fl->proto;
>> diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
>> index 89884a4..60c78cf 100644
>> --- a/net/ipv6/xfrm6_state.c
>> +++ b/net/ipv6/xfrm6_state.c
>> @@ -34,6 +34,7 @@ __xfrm6_init_tempsel(struct xfrm_state *x, struct flowi *fl,
>> x->sel.dport_mask = htons(0xffff);
>> x->sel.sport = xfrm_flowi_sport(fl);
>> x->sel.sport_mask = htons(0xffff);
>> + x->sel.family = AF_INET6;
>> x->sel.prefixlen_d = 128;
>> x->sel.prefixlen_s = 128;
>> x->sel.proto = fl->proto;
>
> Cheers,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector
2008-11-04 11:46 ` Arnaud Ebalard
@ 2008-11-04 11:52 ` Herbert Xu
2008-11-04 22:49 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2008-11-04 11:52 UTC (permalink / raw)
To: Arnaud Ebalard
Cc: David Miller, netdev, Andreas Steffen, Martin Willi,
Kazunori MIYAZAWA
On Tue, Nov 04, 2008 at 12:46:44PM +0100, Arnaud Ebalard wrote:
>
> Sorry Herbert, my initial comment was misleading: the family is not set
> in the selectors provided in the *XFRM_MSG_ACQUIRE*, which is not MIPv6
> related. I could check again, but I think the patch below will impact
> all native key managers. Or did I miss something and there is a specific
> reason why MIPv6 folks may be impacted?
Indeed, you're right. I was thinking of SA creation.
> >> Reported-by: Andreas Steffen <andreas.steffen@strongswan.org>
> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector
2008-11-04 11:52 ` Herbert Xu
@ 2008-11-04 22:49 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2008-11-04 22:49 UTC (permalink / raw)
To: herbert; +Cc: arno, netdev, andreas.steffen, martin, kazunori
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 4 Nov 2008 19:52:28 +0800
> On Tue, Nov 04, 2008 at 12:46:44PM +0100, Arnaud Ebalard wrote:
> >
> > Sorry Herbert, my initial comment was misleading: the family is not set
> > in the selectors provided in the *XFRM_MSG_ACQUIRE*, which is not MIPv6
> > related. I could check again, but I think the patch below will impact
> > all native key managers. Or did I miss something and there is a specific
> > reason why MIPv6 folks may be impacted?
>
> Indeed, you're right. I was thinking of SA creation.
>
> > >> Reported-by: Andreas Steffen <andreas.steffen@strongswan.org>
> > >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied to net-2.6, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-11-04 22:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-04 10:24 [PATCH] Have af-specific init_tempsel() initialize family field of temporary selector Arnaud Ebalard
2008-11-04 11:24 ` Herbert Xu
2008-11-04 11:46 ` Arnaud Ebalard
2008-11-04 11:52 ` Herbert Xu
2008-11-04 22:49 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).