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]

Date:	Fri, 19 Mar 2010 16:29:09 +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 09:48:17AM +0200, Timo Teräs wrote:
>
> But flow_cache_lookup disables pre-emption until _put is called.
> So it should work. Would there be a cleaner way?

Previously the flow cache returned a policy directly which works
because whenever we modify that policy we'd take the appropriate
lock.

Your patch changes it so that it now returns an fce.  But nothing
is guarding the code that modifies fce.  So two CPUs may end up
modifying the same fce.

However, it would appear that this race could be harmless, provided
that you are careful about dereferencing fce->policy and fce->dst.

IOW, this is not OK

	if (fce->policy)
		use fce->policy;

and this should work

	policy = fce->policy;
	if (policy)
		use policy;

Actually on second tought, even this isn't totally safe.  Who
is taking a reference count on the policy and dst? I see a ref
count on the fce, but nothing on fce->dst and fce->policy.  Do
you have an implicit reference on them?

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