netdev - Re: [PATCH] xfrm: cache bundle lookup results in flow cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100319084717.GA23567@gondor.apana.org.au>
Date:	Fri, 19 Mar 2010 16:47:17 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Timo Teräs <timo.teras@....fi>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] xfrm: cache bundle lookup results in flow cache

On Fri, Mar 19, 2010 at 10:37:58AM +0200, Timo Teräs wrote:
>
> But I changed that. the flow cache now does *not* call local_bh_enable
> if it returns something. This is deferred until corresponding _put
> call. So bh's are disable while we are touching the lookup results.

I'm sorry but making a function like flow_cache_lookup return with
BH disabled is just wrong!

> It'd probably make sense to remove that. And require _lookup to
> be called with bh disabled so it's more obvious that bh's are
> disabled when touching the cache entry.

That would be better but it's still hacky.  Proper reference
counting like we had before would be my preference.

> Not a race. We need to keep bh's disabled while touching fce
> for various reasons.

What are those reasons (apart from this race)?

> Noone. When policy and dst is on cache there's no reference.
> The cache generation id's ensure that the object exists when
> they are in cache. It might make sense to add references to
> both objects and do a BUG_ON if the flow cache flusher would
> need to delete an object. I guess this would be the proper
> way, since that's how the dst stuff works too.

The cache genid is not enough:

CPU1			CPU2
check genid == OK
			update genid
			kill policy
			kfree on policy
use policy == BOOM

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html