netdev - Re: [PATCHv3 net-next-2.6 3/5] XFRM,IPv6: Add IRO src/dst address remapping XFRM types and i/o handlers

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: <20101003151202.GA11963@gondor.apana.org.au>
Date:	Sun, 3 Oct 2010 23:12:02 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Arnaud Ebalard <arno@...isbad.org>
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

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 :)

>    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.

Cheers,
-- 
Email: Herbert Xu <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