diff mbox series

[net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states

Message ID 8a3e5a49-5906-b6a6-beb7-0479bc64dcd0@secunet.com
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series [net] xfrm: Interpret XFRM_INF as 32 bit value for non-ESN states | expand

Commit Message

Thomas Egerer Jan. 27, 2020, 2:31 p.m. UTC
Currently, when left unconfigured, hard and soft packet limit are set to
XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
of creating an expire event, the states become unusable and increase
their respective 'state expired' counter in the xfrm statistics. The
only way for them to actually expire is based on their lifetime limits.

This patch reduces the packet limit of non-ESN states with XFRM_INF as
their soft/hard packet limit to their maximum achievable sequence
number in order to trigger an expire, which can then be used by an IKE
daemon to reestablish the connection.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
---
 net/xfrm/xfrm_user.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

David Miller Jan. 28, 2020, 9:54 a.m. UTC | #1
From: Thomas Egerer <thomas.egerer@secunet.com>
Date: Mon, 27 Jan 2020 15:31:14 +0100

> Currently, when left unconfigured, hard and soft packet limit are set to
> XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
> their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
> of creating an expire event, the states become unusable and increase
> their respective 'state expired' counter in the xfrm statistics. The
> only way for them to actually expire is based on their lifetime limits.
> 
> This patch reduces the packet limit of non-ESN states with XFRM_INF as
> their soft/hard packet limit to their maximum achievable sequence
> number in order to trigger an expire, which can then be used by an IKE
> daemon to reestablish the connection.
> 
> Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>

Please always CC: the ipsec maintainers for patches to IPSEC.

Steffen, I assume I will get this from you.

Thanks.

> ---
>  net/xfrm/xfrm_user.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index b88ba45..84d4008 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -505,6 +505,13 @@ static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
>  
>  	if (!x->sel.family && !(p->flags & XFRM_STATE_AF_UNSPEC))
>  		x->sel.family = p->family;
> +
> +	if ((x->props.flags & XFRM_STATE_ESN) == 0 {
> +		if (x->lft.soft_packet_limit == XFRM_INF)
> +			x->lft.soft_packet_limit == (__u32)~0;
> +		if (x->lft.hard_packet_limit == XFRM_INF)
> +			x->lft.hard_packet_limit == (__u32)~0;
> +	}
>  }
>  
>  /*
> -- 
> 2.6.4
>
Steffen Klassert Jan. 29, 2020, 6:15 p.m. UTC | #2
On Tue, Jan 28, 2020 at 10:54:23AM +0100, David Miller wrote:
> From: Thomas Egerer <thomas.egerer@secunet.com>
> Date: Mon, 27 Jan 2020 15:31:14 +0100
> 
> > Currently, when left unconfigured, hard and soft packet limit are set to
> > XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
> > their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
> > of creating an expire event, the states become unusable and increase
> > their respective 'state expired' counter in the xfrm statistics. The
> > only way for them to actually expire is based on their lifetime limits.
> > 
> > This patch reduces the packet limit of non-ESN states with XFRM_INF as
> > their soft/hard packet limit to their maximum achievable sequence
> > number in order to trigger an expire, which can then be used by an IKE
> > daemon to reestablish the connection.
> > 
> > Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
> 
> Please always CC: the ipsec maintainers for patches to IPSEC.
> 
> Steffen, I assume I will get this from you.

Yes, I have it already in my queue.
Steffen Klassert Jan. 30, 2020, 10:34 a.m. UTC | #3
On Mon, Jan 27, 2020 at 03:31:14PM +0100, Thomas Egerer wrote:
> Currently, when left unconfigured, hard and soft packet limit are set to
> XFRM_INF ((__u64)~0). This can be problematic for non-ESN states, as
> their 'natural' packet limit is 2^32 - 1 packets. When reached, instead
> of creating an expire event, the states become unusable and increase
> their respective 'state expired' counter in the xfrm statistics. The
> only way for them to actually expire is based on their lifetime limits.
> 
> This patch reduces the packet limit of non-ESN states with XFRM_INF as
> their soft/hard packet limit to their maximum achievable sequence
> number in order to trigger an expire, which can then be used by an IKE
> daemon to reestablish the connection.
> 
> Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
> ---
>  net/xfrm/xfrm_user.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index b88ba45..84d4008 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -505,6 +505,13 @@ static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
>  
>  	if (!x->sel.family && !(p->flags & XFRM_STATE_AF_UNSPEC))
>  		x->sel.family = p->family;
> +
> +	if ((x->props.flags & XFRM_STATE_ESN) == 0 {

You need one more close bracket here:

/home/klassert/git/ipsec/net/xfrm/xfrm_user.c: In function ‘copy_from_user_state’:
/home/klassert/git/ipsec/net/xfrm/xfrm_user.c:509:45: error: expected ‘)’ before ‘{’ token
  if ((x->props.flags & XFRM_STATE_ESN) == 0 {
                                             ^
/home/klassert/git/ipsec/net/xfrm/xfrm_user.c:515:1: error: expected expression before ‘}’ token
 }
 ^

Please fix and resend.

Thanks.
Thomas Egerer Jan. 30, 2020, 11:52 a.m. UTC | #4
Hello Steffen,

On 1/30/20 11:34 AM, Steffen Klassert wrote:
> On Mon, Jan 27, 2020 at 03:31:14PM +0100, Thomas Egerer wrote:
> 
> You need one more close bracket here:
> 
> /home/klassert/git/ipsec/net/xfrm/xfrm_user.c: In function ‘copy_from_user_state’:
> /home/klassert/git/ipsec/net/xfrm/xfrm_user.c:509:45: error: expected ‘)’ before ‘{’ token
>   if ((x->props.flags & XFRM_STATE_ESN) == 0 {
>                                              ^
> /home/klassert/git/ipsec/net/xfrm/xfrm_user.c:515:1: error: expected expression before ‘}’ token
>  }
>  ^
> 
> Please fix and resend.
You're right. I wonder how that could have happened. I will rebuild
and make sure, I tested the kernel version including the patch and
not the one without.


Thomas
diff mbox series

Patch

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b88ba45..84d4008 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -505,6 +505,13 @@  static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
 
 	if (!x->sel.family && !(p->flags & XFRM_STATE_AF_UNSPEC))
 		x->sel.family = p->family;
+
+	if ((x->props.flags & XFRM_STATE_ESN) == 0 {
+		if (x->lft.soft_packet_limit == XFRM_INF)
+			x->lft.soft_packet_limit == (__u32)~0;
+		if (x->lft.hard_packet_limit == XFRM_INF)
+			x->lft.hard_packet_limit == (__u32)~0;
+	}
 }
 
 /*