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
| ||
|
Message-ID: <87lj6fvuhz.fsf@small.ssi.corp> Date: Sun, 03 Oct 2010 23:25:44 +0200 From: arno@...isbad.org (Arnaud Ebalard) To: Herbert Xu <herbert@...dor.apana.org.au> Cc: David Miller <davem@...emloft.net>, eric.dumazet@...il.com, yoshfuji@...ux-ipv6.org, netdev@...r.kernel.org Subject: Re: [PATCHv3 net-next-2.6 3/5] XFRM,IPv6: Add IRO src/dst address remapping XFRM types and i/o handlers Hello, Herbert Xu <herbert@...dor.apana.org.au> writes: > On Sun, Oct 03, 2010 at 03:41:04PM +0200, Arnaud Ebalard wrote: >> >> After your reply, I took a (too long) look at the history of >> xfrm6_input_addr() to understand why it is as it is. If it can spare you >> some time, here is what I think happened: > > ... > >> - Then, as you wrote, the state lock was moved in all input handlers >> (commit 0ebea8ef, Nov 13 2007), including RH2/HAO ones: > > ... > >> With that commit, I think a deadlock was introduced in MIPv6 code >> because xfm6_input_addr() was left unchanged, i.e. x->type->input() >> was called with the lock held. Am I correct? >> >> - The code of xfrm6_input_addr() was then optimized by commit a002c6fd >> in such a way that x->type->input() was then put outside the >> protection of the lock, which (if I am not mistaken) removed the >> deadlock: > > ... > >> I don't know if this is was intentional. > > Indeed MIPv6 was completely out of action for three months and > nobody noticed :) hehe ;-) Just to correct a missing waypoint in my history, which is in fact the real fix for the deadlock: commit 9473e1f631de339c50bde1e3bd09e1045fe90fd5 Author: Masahide NAKAMURA <nakam@...ux-ipv6.org> Date: Thu Dec 20 20:41:57 2007 -0800 [XFRM] MIPv6: Fix to input RO state correctly. Disable spin_lock during xfrm_type.input() function. Follow design as IPsec inbound does. Signed-off-by: Masahide NAKAMURA <nakam@...ux-ipv6.org> Signed-off-by: David S. Miller <davem@...emloft.net> >> But the main question remains on the position of the lock. Here, >> checks are done on the status of the state, lock is released, >> reacquired in the input handler to do additional check and then >> released again, to be reacquired later in the function to act on >> statistics. Is my reading of the code correct? > > When I moved the lock down I chose the safest option and added > it to every single input function. So it may well be the case > that the lock isn't needed at all on the MIPv6 path. I don't have any technical argument to support the removal of the locks, i.e. don't see what would prevent changes during the check. I will try and spend more time on it, but meanwhile I think it's safe to keep things the way they are. Thanks for your time, Herbert. Cheers, a+ -- 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
Powered by blists - more mailing lists