Subject: Re: [PATCH] xfrm: Fix double dst_release() in xfrm_lookup() -EREMOTE case

From: Timo Teräs <[email-masked]>
Date: Thu, 08 Apr 2010 15:39:38 +0300

Mark Brown wrote:
> Commit 80c802 ("xfrm: cache bundles instead of policies for outgoing
> flows") changed __xfrm_policy() to call dst_release() when returning
> -EREMOTE. In the case where this is called from xfrm_lookup() a double
> call to dst_release() would result due to the existing dst_release()
> there. Remove the dst_release() in xfrm_lookup().
> 
> Signed-off-by: Mark Brown <[email-masked]>
> ---
> 
> I'm not sure if this is correct or not - there may a reference been
> taken earlier in __xfrm_lookup() that's being dropped but I didn't spot
> it.

This is not correct.

The only case we return -EREMOTE from __xfrm_policy() is at:
                if (net->xfrm.sysctl_larval_drop) {
                        /* EREMOTE tells the caller to generate
                         * a one-shot blackhole route. */
                        dst_release(dst);
                        xfrm_pols_put(pols, num_pols);
                        XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
                        return -EREMOTE;
                }

It drops the inner xfrm_dst returned from flow cache lookup / resolver.
xfrm_lookup() will drop the original dst (which is not xfrm_dst).

This semantics is important because __xfrm_lookup() is also called
from other places, that do other things when they get -EREMOTE.

- TImo