[CRIU] [PATCH] restore: unlock network before disabling repair mode on sockets

[Pavel Emelyanov]

On 07/15/2013 04:55 PM, Andrew Vagin wrote:
> On Mon, Jul 15, 2013 at 04:48:13PM +0400, Pavel Emelyanov wrote:
>> On 07/15/2013 03:41 PM, Andrew Vagin wrote:
>>> On Mon, Jul 15, 2013 at 03:20:35PM +0400, Andrew Vagin wrote:
>>>> On Mon, Jul 15, 2013 at 03:07:15PM +0400, Pavel Emelyanov wrote:
>>>>> On 07/14/2013 05:47 PM, Andrey Vagin wrote:
>>>>>> Window probe is sent during disabling repair mode on a socket, so
>>>>>> network must be unlocked in this moment.
>>>>>
>>>>> I thought we've discussed this thing already and kinda fixed one.
>>>>
>>>> I have a similar remembrances, but look at the code
>>>
>>> commit c27ff2baac506b84c08f3d4e93e79adb5b4affa6
>>> Author: Andrey Vagin <avagin at openvz.org>
>>> Date:   Mon Sep 17 20:02:57 2012 +0400
>>>
>>>     tcp: unset TCP_REPAIR at the last moment after unlocking network
>>> (v2)
>>>     
>>>     TCP_REPAIR should be droppet when a network is unlocked.
>>>     A network should be unlocked at the last moment, because
>>>     after this moment restore must not failed, otherwise a state of
>>>     a tcp connection can be changed and a state of one side in our image
>>>     will be invalid.
>>>     
>>>     v2: use xremalloc instead of mmap and remmap
>>>     
>>>     Signed-off-by: Andrey Vagin <avagin at openvz.org>
>>>     Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
>>>
>>> and than it was broken again by another commit:
>>>
>>> commit a34057c1925ec421eae1d7c52f02674209208372
>>> Author: Andrey Vagin <avagin at openvz.org>
>>> Date:   Fri Apr 19 15:58:50 2013 +0400
>>>
>>>     restore: add a synchronisation point after restoring credentials
>>>     
>>>     For security reason processes can be resumed only when all
>>>     credentials are restored. Otherwise someone can attach to a
>>>     process, which are not restored credentials yet and execute
>>>     some code.
>>>     
>>>     https://bugzilla.openvz.org/show_bug.cgi?id=2561
>>>     
>>>     Signed-off-by: Andrey Vagin <avagin at openvz.org>
>>>     Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
>>
>> OK. Wouldn't it be easier to move the call to repair-off from restorer lower?
> 
> We can not do that, because capabilities should be restored after
> disabling repair mode.

OK. Doing the network_unlock in the place you provide cannot be done
either, since any fail after that should be handled, but it would be
impossible to roll-back the unlocked_network.

The sequence should be

do_everything_that_can_fail()
unlock_network()
restore_creads()
resume()

Thanks,
Pavel