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

[Andrew Vagin]

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>

> Currently network_unlock and rst_tcp_repair_off are called on the same
> stage, so we have a race condition.
> 
> 
> > What's wrong?
> > 
> > > https://bugzilla.openvz.org/show_bug.cgi?id=2670
> > > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > > ---
> > >  cr-restore.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/cr-restore.c b/cr-restore.c
> > > index 1d014e1..d7f1e31 100644
> > > --- a/cr-restore.c
> > > +++ b/cr-restore.c
> > > @@ -1300,6 +1300,13 @@ static int restore_root_task(struct pstree_item *init)
> > >  		goto out;
> > >  
> > >  	pr_info("Wait until all tasks are restored\n");
> > > +	ret = restore_wait_inprogress_tasks();
> > > +	if (ret < 0)
> > > +		goto out;
> > > +
> > > +	/* Unlock network before disabling repair mode on sockets */
> > > +	network_unlock();
> > > +
> > >  	ret = restore_switch_stage(CR_STATE_RESTORE_CREDS);
> > >  	if (ret < 0)
> > >  		goto out;
> > > @@ -1313,7 +1320,6 @@ static int restore_root_task(struct pstree_item *init)
> > >  		goto out;
> > >  	}
> > >  
> > > -	network_unlock();
> > >  out:
> > >  	if (ret < 0) {
> > >  		struct pstree_item *pi;
> > > 
> > 
> >