* pull request (net): ipsec 2013-10-09
@ 2013-10-09 10:59 Steffen Klassert
2013-10-09 10:59 ` [PATCH 1/7] xfrm: Fix replay size checking on async events Steffen Klassert
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
1) We used the wrong netlink attribute to verify the
lenght of the replay window on async events. Fix this by
using the right netlink attribute.
2) Policy lookups can not match the output interface on forwarding.
Add the needed informations to the flow informations.
3) We update the pmtu when we receive a ICMPV6_DEST_UNREACH message
on IPsec with ipv6. This is wrong and leads to strange fragmented
packets, only ICMPV6_PKT_TOOBIG messages should update the pmtu.
Fix this by removing the ICMPV6_DEST_UNREACH check from the IPsec
protocol error handlers.
4) The legacy IPsec anti replay mechanism supports anti replay
windows up to 32 packets. If a user requests for a bigger
anti replay window, we use 32 packets but pretend that we use
the requested window size. Fix from Fan Du.
5) If asynchronous events are enabled and replay_maxdiff is set to
zero, we generate an async event for every received packet instead
of checking whether a timeout occurred. Fix from Thomas Egerer.
6) Policies need a refcount when the state resolution timer is armed.
Otherwise the timer can fire after the policy is deleted.
7) We might dreference a NULL pointer if the hold_queue is empty,
add a check to avoid this.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit 73a695f8572e4c46a2aecdbb63f26f36a43e6873:
cxgb4: remove workqueue when driver registration fails (2013-09-15 22:28:58 -0400)
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 2bb53e2557964c2c5368a0392cf3b3b63a288cd0:
xfrm: check for a vaild skb in xfrm_policy_queue_process (2013-10-08 10:49:51 +0200)
----------------------------------------------------------------
Fan Du (1):
xfrm: Guard IPsec anti replay window against replay bitmap
Steffen Klassert (5):
xfrm: Fix replay size checking on async events
xfrm: Decode sessions with output interface.
ipsec: Don't update the pmtu on ICMPV6_DEST_UNREACH
xfrm: Add refcount handling to queued policies
xfrm: check for a vaild skb in xfrm_policy_queue_process
Thomas Egerer (1):
xfrm: Fix aevent generation for each received packet
net/ipv4/xfrm4_policy.c | 1 +
net/ipv6/ah6.c | 3 +--
net/ipv6/esp6.c | 3 +--
net/ipv6/ipcomp6.c | 3 +--
net/ipv6/xfrm6_policy.c | 1 +
net/key/af_key.c | 3 ++-
net/xfrm/xfrm_policy.c | 28 ++++++++++++++++++------
net/xfrm/xfrm_replay.c | 54 ++++++++++++++++++++++++-----------------------
net/xfrm/xfrm_user.c | 5 +++--
9 files changed, 59 insertions(+), 42 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/7] xfrm: Fix replay size checking on async events
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 10:59 ` [PATCH 2/7] xfrm: Decode sessions with output interface Steffen Klassert
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
We pass the wrong netlink attribute to xfrm_replay_verify_len().
It should be XFRMA_REPLAY_ESN_VAL and not XFRMA_REPLAY_VAL as
we currently doing. This causes memory corruptions if the
replay esn attribute has incorrect length. Fix this by passing
the right attribute to xfrm_replay_verify_len().
Reported-by: Michael Rossberg <michael.rossberg@tu-ilmenau.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 3f565e4..4b26cee 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1856,7 +1856,7 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
if (x->km.state != XFRM_STATE_VALID)
goto out;
- err = xfrm_replay_verify_len(x->replay_esn, rp);
+ err = xfrm_replay_verify_len(x->replay_esn, re);
if (err)
goto out;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/7] xfrm: Decode sessions with output interface.
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
2013-10-09 10:59 ` [PATCH 1/7] xfrm: Fix replay size checking on async events Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 10:59 ` [PATCH 3/7] ipsec: Don't update the pmtu on ICMPV6_DEST_UNREACH Steffen Klassert
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
The output interface matching does not work on forward
policy lookups, the output interface of the flowi is
always 0. Fix this by setting the output interface when
we decode the session.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_policy.c | 1 +
net/ipv6/xfrm6_policy.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 9a459be..ccde542 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -107,6 +107,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
memset(fl4, 0, sizeof(struct flowi4));
fl4->flowi4_mark = skb->mark;
+ fl4->flowi4_oif = skb_dst(skb)->dev->ifindex;
if (!ip_is_fragment(iph)) {
switch (iph->protocol) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 23ed03d..08ed277 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -138,6 +138,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
memset(fl6, 0, sizeof(struct flowi6));
fl6->flowi6_mark = skb->mark;
+ fl6->flowi6_oif = skb_dst(skb)->dev->ifindex;
fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/7] ipsec: Don't update the pmtu on ICMPV6_DEST_UNREACH
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
2013-10-09 10:59 ` [PATCH 1/7] xfrm: Fix replay size checking on async events Steffen Klassert
2013-10-09 10:59 ` [PATCH 2/7] xfrm: Decode sessions with output interface Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 10:59 ` [PATCH 4/7] xfrm: Guard IPsec anti replay window against replay bitmap Steffen Klassert
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
Currently we update the pmtu in the IPsec protocol error handlers
if icmpv6 message type is either ICMPV6_DEST_UNREACH or
ICMPV6_PKT_TOOBIG. Updating the pmtu on ICMPV6_DEST_UNREACH
is wrong in any case, it causes strangely fragmented packets.
Only ICMPV6_PKT_TOOBIG signalizes pmtu discovery, so remove the
ICMPV6_DEST_UNREACH check in the IPsec protocol error handlers.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/ah6.c | 3 +--
net/ipv6/esp6.c | 3 +--
net/ipv6/ipcomp6.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 73784c3..82e1da3 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -618,8 +618,7 @@ static void ah6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
struct ip_auth_hdr *ah = (struct ip_auth_hdr*)(skb->data+offset);
struct xfrm_state *x;
- if (type != ICMPV6_DEST_UNREACH &&
- type != ICMPV6_PKT_TOOBIG &&
+ if (type != ICMPV6_PKT_TOOBIG &&
type != NDISC_REDIRECT)
return;
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index d3618a7..e67e63f 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -436,8 +436,7 @@ static void esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
struct ip_esp_hdr *esph = (struct ip_esp_hdr *)(skb->data + offset);
struct xfrm_state *x;
- if (type != ICMPV6_DEST_UNREACH &&
- type != ICMPV6_PKT_TOOBIG &&
+ if (type != ICMPV6_PKT_TOOBIG &&
type != NDISC_REDIRECT)
return;
diff --git a/net/ipv6/ipcomp6.c b/net/ipv6/ipcomp6.c
index 5636a91..ce507d9 100644
--- a/net/ipv6/ipcomp6.c
+++ b/net/ipv6/ipcomp6.c
@@ -64,8 +64,7 @@ static void ipcomp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
(struct ip_comp_hdr *)(skb->data + offset);
struct xfrm_state *x;
- if (type != ICMPV6_DEST_UNREACH &&
- type != ICMPV6_PKT_TOOBIG &&
+ if (type != ICMPV6_PKT_TOOBIG &&
type != NDISC_REDIRECT)
return;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/7] xfrm: Guard IPsec anti replay window against replay bitmap
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
` (2 preceding siblings ...)
2013-10-09 10:59 ` [PATCH 3/7] ipsec: Don't update the pmtu on ICMPV6_DEST_UNREACH Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 10:59 ` [PATCH 5/7] xfrm: Fix aevent generation for each received packet Steffen Klassert
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Fan Du <fan.du@windriver.com>
For legacy IPsec anti replay mechanism:
bitmap in struct xfrm_replay_state could only provide a 32 bits
window size limit in current design, thus user level parameter
sadb_sa_replay should honor this limit, otherwise misleading
outputs("replay=244") by setkey -D will be:
192.168.25.2 192.168.22.2
esp mode=transport spi=147561170(0x08cb9ad2) reqid=0(0x00000000)
E: aes-cbc 9a8d7468 7655cf0b 719d27be b0ddaac2
A: hmac-sha1 2d2115c2 ebf7c126 1c54f186 3b139b58 264a7331
seq=0x00000000 replay=244 flags=0x00000000 state=mature
created: Sep 17 14:00:00 2013 current: Sep 17 14:00:22 2013
diff: 22(s) hard: 30(s) soft: 26(s)
last: Sep 17 14:00:00 2013 hard: 0(s) soft: 0(s)
current: 1408(bytes) hard: 0(bytes) soft: 0(bytes)
allocated: 22 hard: 0 soft: 0
sadb_seq=1 pid=4854 refcnt=0
192.168.22.2 192.168.25.2
esp mode=transport spi=255302123(0x0f3799eb) reqid=0(0x00000000)
E: aes-cbc 6485d990 f61a6bd5 e5660252 608ad282
A: hmac-sha1 0cca811a eb4fa893 c47ae56c 98f6e413 87379a88
seq=0x00000000 replay=244 flags=0x00000000 state=mature
created: Sep 17 14:00:00 2013 current: Sep 17 14:00:22 2013
diff: 22(s) hard: 30(s) soft: 26(s)
last: Sep 17 14:00:00 2013 hard: 0(s) soft: 0(s)
current: 1408(bytes) hard: 0(bytes) soft: 0(bytes)
allocated: 22 hard: 0 soft: 0
sadb_seq=0 pid=4854 refcnt=0
And also, optimizing xfrm_replay_check window checking by setting the
desirable x->props.replay_window with only doing the comparison once
for all when xfrm_state is first born.
Signed-off-by: Fan Du <fan.du@windriver.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/key/af_key.c | 3 ++-
net/xfrm/xfrm_replay.c | 3 +--
net/xfrm/xfrm_user.c | 3 ++-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9d58537..911ef03 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1098,7 +1098,8 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
x->id.proto = proto;
x->id.spi = sa->sadb_sa_spi;
- x->props.replay_window = sa->sadb_sa_replay;
+ x->props.replay_window = min_t(unsigned int, sa->sadb_sa_replay,
+ (sizeof(x->replay.bitmap) * 8));
if (sa->sadb_sa_flags & SADB_SAFLAGS_NOECN)
x->props.flags |= XFRM_STATE_NOECN;
if (sa->sadb_sa_flags & SADB_SAFLAGS_DECAP_DSCP)
diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index 8dafe6d3..eeca388 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -129,8 +129,7 @@ static int xfrm_replay_check(struct xfrm_state *x,
return 0;
diff = x->replay.seq - seq;
- if (diff >= min_t(unsigned int, x->props.replay_window,
- sizeof(x->replay.bitmap) * 8)) {
+ if (diff >= x->props.replay_window) {
x->stats.replay_window++;
goto err;
}
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 4b26cee..f964d4c 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -446,7 +446,8 @@ static void copy_from_user_state(struct xfrm_state *x, struct xfrm_usersa_info *
memcpy(&x->sel, &p->sel, sizeof(x->sel));
memcpy(&x->lft, &p->lft, sizeof(x->lft));
x->props.mode = p->mode;
- x->props.replay_window = p->replay_window;
+ x->props.replay_window = min_t(unsigned int, p->replay_window,
+ sizeof(x->replay.bitmap) * 8);
x->props.reqid = p->reqid;
x->props.family = p->family;
memcpy(&x->props.saddr, &p->saddr, sizeof(x->props.saddr));
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/7] xfrm: Fix aevent generation for each received packet
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
` (3 preceding siblings ...)
2013-10-09 10:59 ` [PATCH 4/7] xfrm: Guard IPsec anti replay window against replay bitmap Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 10:59 ` [PATCH 6/7] xfrm: Add refcount handling to queued policies Steffen Klassert
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Thomas Egerer <thomas.egerer@secunet.com>
If asynchronous events are enabled for a particular netlink socket,
the notify function is called by the advance function. The notify
function creates and dispatches a km_event if a replay timeout occurred,
or at least replay_maxdiff packets have been received since the last
asynchronous event has been sent. The function is supposed to return if
neither of the two events were detected for a state, or replay_maxdiff
is equal to zero.
Replay_maxdiff is initialized in xfrm_state_construct to the value of
the xfrm.sysctl_aevent_rseqth (2 by default), and updated if for a state
if the netlink attribute XFRMA_REPLAY_THRESH is set.
If, however, replay_maxdiff is set to zero, then all of the three notify
implementations perform a break from the switch statement instead of
checking whether a timeout occurred, and -- if not -- return. As a
result an asynchronous event is generated for every replay update of a
state that has a zero replay_maxdiff value.
This patch modifies the notify functions such that they immediately
return if replay_maxdiff has the value zero, unless a timeout occurred.
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_replay.c | 51 +++++++++++++++++++++++++-----------------------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index eeca388..dab57da 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -61,9 +61,9 @@ static void xfrm_replay_notify(struct xfrm_state *x, int event)
switch (event) {
case XFRM_REPLAY_UPDATE:
- if (x->replay_maxdiff &&
- (x->replay.seq - x->preplay.seq < x->replay_maxdiff) &&
- (x->replay.oseq - x->preplay.oseq < x->replay_maxdiff)) {
+ if (!x->replay_maxdiff ||
+ ((x->replay.seq - x->preplay.seq < x->replay_maxdiff) &&
+ (x->replay.oseq - x->preplay.oseq < x->replay_maxdiff))) {
if (x->xflags & XFRM_TIME_DEFER)
event = XFRM_REPLAY_TIMEOUT;
else
@@ -301,9 +301,10 @@ static void xfrm_replay_notify_bmp(struct xfrm_state *x, int event)
switch (event) {
case XFRM_REPLAY_UPDATE:
- if (x->replay_maxdiff &&
- (replay_esn->seq - preplay_esn->seq < x->replay_maxdiff) &&
- (replay_esn->oseq - preplay_esn->oseq < x->replay_maxdiff)) {
+ if (!x->replay_maxdiff ||
+ ((replay_esn->seq - preplay_esn->seq < x->replay_maxdiff) &&
+ (replay_esn->oseq - preplay_esn->oseq
+ < x->replay_maxdiff))) {
if (x->xflags & XFRM_TIME_DEFER)
event = XFRM_REPLAY_TIMEOUT;
else
@@ -352,28 +353,30 @@ static void xfrm_replay_notify_esn(struct xfrm_state *x, int event)
switch (event) {
case XFRM_REPLAY_UPDATE:
- if (!x->replay_maxdiff)
- break;
-
- if (replay_esn->seq_hi == preplay_esn->seq_hi)
- seq_diff = replay_esn->seq - preplay_esn->seq;
- else
- seq_diff = ~preplay_esn->seq + replay_esn->seq + 1;
-
- if (replay_esn->oseq_hi == preplay_esn->oseq_hi)
- oseq_diff = replay_esn->oseq - preplay_esn->oseq;
- else
- oseq_diff = ~preplay_esn->oseq + replay_esn->oseq + 1;
-
- if (seq_diff < x->replay_maxdiff &&
- oseq_diff < x->replay_maxdiff) {
+ if (x->replay_maxdiff) {
+ if (replay_esn->seq_hi == preplay_esn->seq_hi)
+ seq_diff = replay_esn->seq - preplay_esn->seq;
+ else
+ seq_diff = ~preplay_esn->seq + replay_esn->seq
+ + 1;
- if (x->xflags & XFRM_TIME_DEFER)
- event = XFRM_REPLAY_TIMEOUT;
+ if (replay_esn->oseq_hi == preplay_esn->oseq_hi)
+ oseq_diff = replay_esn->oseq
+ - preplay_esn->oseq;
else
- return;
+ oseq_diff = ~preplay_esn->oseq
+ + replay_esn->oseq + 1;
+
+ if (seq_diff >= x->replay_maxdiff ||
+ oseq_diff >= x->replay_maxdiff)
+ break;
}
+ if (x->xflags & XFRM_TIME_DEFER)
+ event = XFRM_REPLAY_TIMEOUT;
+ else
+ return;
+
break;
case XFRM_REPLAY_TIMEOUT:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 6/7] xfrm: Add refcount handling to queued policies
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
` (4 preceding siblings ...)
2013-10-09 10:59 ` [PATCH 5/7] xfrm: Fix aevent generation for each received packet Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 10:59 ` [PATCH 7/7] xfrm: check for a vaild skb in xfrm_policy_queue_process Steffen Klassert
2013-10-09 17:44 ` pull request (net): ipsec 2013-10-09 David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
We need to ensure that policies can't go away as long as the hold timer
is armed, so take a refcont when we arm the timer and drop one if we
delete it.
Bug was introduced with git commit a0073fe18 ("xfrm: Add a state
resolution packet queue")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index ed38d5d..5f9be97 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -334,7 +334,8 @@ static void xfrm_policy_kill(struct xfrm_policy *policy)
atomic_inc(&policy->genid);
- del_timer(&policy->polq.hold_timer);
+ if (del_timer(&policy->polq.hold_timer))
+ xfrm_pol_put(policy);
xfrm_queue_purge(&policy->polq.hold_queue);
if (del_timer(&policy->timer))
@@ -589,7 +590,8 @@ static void xfrm_policy_requeue(struct xfrm_policy *old,
spin_lock_bh(&pq->hold_queue.lock);
skb_queue_splice_init(&pq->hold_queue, &list);
- del_timer(&pq->hold_timer);
+ if (del_timer(&pq->hold_timer))
+ xfrm_pol_put(old);
spin_unlock_bh(&pq->hold_queue.lock);
if (skb_queue_empty(&list))
@@ -600,7 +602,8 @@ static void xfrm_policy_requeue(struct xfrm_policy *old,
spin_lock_bh(&pq->hold_queue.lock);
skb_queue_splice(&list, &pq->hold_queue);
pq->timeout = XFRM_QUEUE_TMO_MIN;
- mod_timer(&pq->hold_timer, jiffies);
+ if (!mod_timer(&pq->hold_timer, jiffies))
+ xfrm_pol_hold(new);
spin_unlock_bh(&pq->hold_queue.lock);
}
@@ -1787,8 +1790,9 @@ static void xfrm_policy_queue_process(unsigned long arg)
goto purge_queue;
pq->timeout = pq->timeout << 1;
- mod_timer(&pq->hold_timer, jiffies + pq->timeout);
- return;
+ if (!mod_timer(&pq->hold_timer, jiffies + pq->timeout))
+ xfrm_pol_hold(pol);
+ goto out;
}
dst_release(dst);
@@ -1819,11 +1823,14 @@ static void xfrm_policy_queue_process(unsigned long arg)
err = dst_output(skb);
}
+out:
+ xfrm_pol_put(pol);
return;
purge_queue:
pq->timeout = 0;
xfrm_queue_purge(&pq->hold_queue);
+ xfrm_pol_put(pol);
}
static int xdst_queue_output(struct sk_buff *skb)
@@ -1831,7 +1838,8 @@ static int xdst_queue_output(struct sk_buff *skb)
unsigned long sched_next;
struct dst_entry *dst = skb_dst(skb);
struct xfrm_dst *xdst = (struct xfrm_dst *) dst;
- struct xfrm_policy_queue *pq = &xdst->pols[0]->polq;
+ struct xfrm_policy *pol = xdst->pols[0];
+ struct xfrm_policy_queue *pq = &pol->polq;
if (pq->hold_queue.qlen > XFRM_MAX_QUEUE_LEN) {
kfree_skb(skb);
@@ -1850,10 +1858,12 @@ static int xdst_queue_output(struct sk_buff *skb)
if (del_timer(&pq->hold_timer)) {
if (time_before(pq->hold_timer.expires, sched_next))
sched_next = pq->hold_timer.expires;
+ xfrm_pol_put(pol);
}
__skb_queue_tail(&pq->hold_queue, skb);
- mod_timer(&pq->hold_timer, sched_next);
+ if (!mod_timer(&pq->hold_timer, sched_next))
+ xfrm_pol_hold(pol);
spin_unlock_bh(&pq->hold_queue.lock);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 7/7] xfrm: check for a vaild skb in xfrm_policy_queue_process
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
` (5 preceding siblings ...)
2013-10-09 10:59 ` [PATCH 6/7] xfrm: Add refcount handling to queued policies Steffen Klassert
@ 2013-10-09 10:59 ` Steffen Klassert
2013-10-09 17:44 ` pull request (net): ipsec 2013-10-09 David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2013-10-09 10:59 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
We might dreference a NULL pointer if the hold_queue is empty,
so add a check to avoid this.
Bug was introduced with git commit a0073fe18 ("xfrm: Add a state
resolution packet queue")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5f9be97..76e1873 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1772,6 +1772,10 @@ static void xfrm_policy_queue_process(unsigned long arg)
spin_lock(&pq->hold_queue.lock);
skb = skb_peek(&pq->hold_queue);
+ if (!skb) {
+ spin_unlock(&pq->hold_queue.lock);
+ goto out;
+ }
dst = skb_dst(skb);
sk = skb->sk;
xfrm_decode_session(skb, &fl, dst->ops->family);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: pull request (net): ipsec 2013-10-09
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
` (6 preceding siblings ...)
2013-10-09 10:59 ` [PATCH 7/7] xfrm: check for a vaild skb in xfrm_policy_queue_process Steffen Klassert
@ 2013-10-09 17:44 ` David Miller
7 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2013-10-09 17:44 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 9 Oct 2013 12:59:04 +0200
> 1) We used the wrong netlink attribute to verify the
> lenght of the replay window on async events. Fix this by
> using the right netlink attribute.
>
> 2) Policy lookups can not match the output interface on forwarding.
> Add the needed informations to the flow informations.
>
> 3) We update the pmtu when we receive a ICMPV6_DEST_UNREACH message
> on IPsec with ipv6. This is wrong and leads to strange fragmented
> packets, only ICMPV6_PKT_TOOBIG messages should update the pmtu.
> Fix this by removing the ICMPV6_DEST_UNREACH check from the IPsec
> protocol error handlers.
>
> 4) The legacy IPsec anti replay mechanism supports anti replay
> windows up to 32 packets. If a user requests for a bigger
> anti replay window, we use 32 packets but pretend that we use
> the requested window size. Fix from Fan Du.
>
> 5) If asynchronous events are enabled and replay_maxdiff is set to
> zero, we generate an async event for every received packet instead
> of checking whether a timeout occurred. Fix from Thomas Egerer.
>
> 6) Policies need a refcount when the state resolution timer is armed.
> Otherwise the timer can fire after the policy is deleted.
>
> 7) We might dreference a NULL pointer if the hold_queue is empty,
> add a check to avoid this.
>
> Please pull or let me know if there are problems.
Pulled, thanks a lot Steffen.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-10-09 17:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-09 10:59 pull request (net): ipsec 2013-10-09 Steffen Klassert
2013-10-09 10:59 ` [PATCH 1/7] xfrm: Fix replay size checking on async events Steffen Klassert
2013-10-09 10:59 ` [PATCH 2/7] xfrm: Decode sessions with output interface Steffen Klassert
2013-10-09 10:59 ` [PATCH 3/7] ipsec: Don't update the pmtu on ICMPV6_DEST_UNREACH Steffen Klassert
2013-10-09 10:59 ` [PATCH 4/7] xfrm: Guard IPsec anti replay window against replay bitmap Steffen Klassert
2013-10-09 10:59 ` [PATCH 5/7] xfrm: Fix aevent generation for each received packet Steffen Klassert
2013-10-09 10:59 ` [PATCH 6/7] xfrm: Add refcount handling to queued policies Steffen Klassert
2013-10-09 10:59 ` [PATCH 7/7] xfrm: check for a vaild skb in xfrm_policy_queue_process Steffen Klassert
2013-10-09 17:44 ` pull request (net): ipsec 2013-10-09 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).