netdev
[Top] [All Lists]

Re: [IPSEC]: Kill nested read lock by deleting xfrm_init_tempsel

To: "David S. Miller" <davem@xxxxxxxxxxxxx>
Subject: Re: [IPSEC]: Kill nested read lock by deleting xfrm_init_tempsel
From: Patrick McHardy <kaber@xxxxxxxxx>
Date: Fri, 22 Apr 2005 01:53:43 +0200
Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>, kuznet@xxxxxxxxxxxxx, jmorris@xxxxxxxxxx, yoshfuji@xxxxxxxxxxxxxx, netdev@xxxxxxxxxxx
In-reply-to: <20050421163526.7a29a76f.davem@xxxxxxxxxxxxx>
References: <20050328233917.GB15369@xxxxxxxxxxxxxxxxxxx> <424B40C2.90304@xxxxxxxxx> <20050331004658.GA26395@xxxxxxxxxxxxxxxxxxx> <20050331212325.5e996432.davem@xxxxxxxxxxxxx> <20050402004956.GA24339@xxxxxxxxxxxxxxxxxxx> <20050401172007.7296eced.davem@xxxxxxxxxxxxx> <20050402020947.GA24998@xxxxxxxxxxxxxxxxxxx> <42501E51.3000401@xxxxxxxxx> <20050405103918.GA24863@xxxxxxxxxxxxxxxxxxx> <4252EEA2.5020107@xxxxxxxxx> <20050406022155.GA12952@xxxxxxxxxxxxxxxxxxx> <20050421163526.7a29a76f.davem@xxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.6) Gecko/20050324 Debian/1.7.6-1
David S. Miller wrote:
On Wed, 6 Apr 2005 12:21:55 +1000
Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:

Thanks.  Just one more issue that I can think of, the check should
only be done when tmpl->id.spi != 0.  Otherwise the presence of
valid states with differing state selectors will prevent new
sessions from starting up.

Is it really worthwhile, right now, to change that tmpl->id.daddr to
daddr?  That seems to be all that Patrick's most recent patch does.

Yes, tmpl->id.daddr might be 0, in which case the destination
of the packet or previous tunnel mode transforms is used. daddr
always contains the correct adress, so we should use it to check
for duplicate SPIs. But as Herbert noted, we shouldn't perform
the check if tmpl->id.spi == 0, so here is a new patch.

Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
===== net/xfrm/xfrm_state.c 1.60 vs edited =====
--- 1.60/net/xfrm/xfrm_state.c  2005-04-01 07:19:54 +02:00
+++ edited/net/xfrm/xfrm_state.c        2005-04-22 01:51:37 +02:00
@@ -357,8 +357,9 @@
 
        x = best;
        if (!x && !error && !acquire_in_progress) {
-               x0 = afinfo->state_lookup(&tmpl->id.daddr, tmpl->id.spi, 
tmpl->id.proto);
-               if (x0 != NULL) {
+               if (tmpl->id.spi &&
+                   (x0 = afinfo->state_lookup(daddr, tmpl->id.spi,
+                                              tmpl->id.proto)) != NULL) {
                        xfrm_state_put(x0);
                        error = -EEXIST;
                        goto out;
<Prev in Thread] Current Thread [Next in Thread>