Re: patch2: del/get byid

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Fri, Apr 29, 2005 at 07:20:46PM -0400, jamal wrote:
>
> @@ -341,17 +341,35 @@
>  {
>       struct xfrm_policy *pol, **p;
>       struct xfrm_policy *delpol = NULL;
> +     struct xfrm_policy *delpol2 = NULL;
> +     struct xfrm_policy *delp = NULL;
>       struct xfrm_policy **newpos = NULL;
> +     int ret = -EINVAL;
> +
> +     if (policy->index)
> +             delpol = xfrm_policy_byid(dir, policy->index, 0);
> +     delpol2 = xfrm_policy_bysel(dir, &policy->selector, 0);
> +
> +     /* must be unique in both index and selector */
> +     if (delpol && delpol2) 
> +             if (delpol != delpol2) 
> +                     goto pol_err;
>  
> +     if (delpol) 
> +             delp = delpol;
> +     else 
> +             delp = delpol2;
> +                     
> +     if (delp && excl) { 
> +             ret = -EEXIST;
> +             goto pol_err;
> +     }
> +
> +     /* insert, sorted by prio*/
>       write_lock_bh(&xfrm_policy_lock);

...

>       write_unlock_bh(&xfrm_policy_lock);
>  
> -     if (delpol) {
> -             xfrm_policy_kill(delpol);
> +     if (delp) {
> +             xfrm_policy_kill(delp);
>       }

This is racy.  The policy could be killed by the timer in the middle.

This is also getting pretty hairy for something that nobody has needed so
far.  Which user-space application is going to use this index thing?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt